Calling functions from Events
-
I'll second JSOP's method. Why cause function call overhead if you don't need to? Probably a moot point, especially if it's a button click, but something like a resize or repaint event could be called thousands of times.
- S 50 cups of coffee and you know it's on! Code, follow, or get out of the way.
Yeah, but I'm usually painting custom controls in layers, so it makes more sense to put each layer in its own method. Then you can turn those layers on and off at will and replace them quite easily. Chances are you're not going to paint anything at 30 frames a second so it doesn't matter that much. If you wanted that kind of performance, I'd look to something other than GDI/GDI+.
A guide to posting questions on CodeProject[^]
Dave Kreskowiak -
Depends on how much they're doing. If it's just a couple lines, and won't be used anywhere else, I stick it in the event handler. If it's more substantial than that, or looks like it belongs in the model layer, or will be called from elsewhere, then I move it to a separate function.
Proud to have finally moved to the A-Ark. Which one are you in?
Author of the Guardians Saga (Sci-Fi/Fantasy novels)i agree... couple of lines, not worth the effort... a lot of lines, probably will make the code a lot cleaner
-
I only do it if the same code can be called from non-event code.
".45 ACP - because shooting twice is just silly" - JSOP, 2010
-----
You can never have too much ammo - unless you're swimming, or on fire. - JSOP, 2010
-----
"Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass." - Dale Earnhardt, 1997good point
-
Not a question, more of a survey. How many people here always separate code from event handlers, especially Form events. I always do it, but it is driving me mad looking at and updating code that doesn't do it, written by a senior engineer. All my event handlers call functions, nothing more.
Craigslist Troll: litaly@comcast.net "I have a theory that the truth is never told during the nine-to-five hours. " — Hunter S. Thompson
Yeah, only if large, have common code, or need to be on a separate thread, otherwise I don't sweat it.
wizardzz wrote:
All my event handlers call functions, nothing more.
That sounds needless, why not simply rename the handlers to whatever the methods are named?
-
The worst is when you see code that calls an event handler, passing EventArgs.Empty, in order to trigger the same behaviour.
Christian Graus Driven to the arms of OSX by Vista. Read my blog to find out how I've worked around bugs in Microsoft tools and frameworks.
I pass
nullinstead. -
Not a question, more of a survey. How many people here always separate code from event handlers, especially Form events. I always do it, but it is driving me mad looking at and updating code that doesn't do it, written by a senior engineer. All my event handlers call functions, nothing more.
Craigslist Troll: litaly@comcast.net "I have a theory that the truth is never told during the nine-to-five hours. " — Hunter S. Thompson
There isn't any one way to split this stuff up and everyone has their own way of understanding it. My rule of thumb is that every chunk of code (function, event handler, whatever) is relatively simple to understand on it's own. Code with too many function calls can be harder to understand then 3,000 line nested if and switch statement monstrosities. If you have to go poking around in 5 or 6 files just to understand what one method does, that's a complete mess. As long as you stay away from the super long or super short clever function extreme, you should be good. As far as your team is concerned, the most important thing is that you guys roughly agree on the basics.
Curvature of the Mind now with 3D
-
Not a question, more of a survey. How many people here always separate code from event handlers, especially Form events. I always do it, but it is driving me mad looking at and updating code that doesn't do it, written by a senior engineer. All my event handlers call functions, nothing more.
Craigslist Troll: litaly@comcast.net "I have a theory that the truth is never told during the nine-to-five hours. " — Hunter S. Thompson
wizardzz wrote:
it is driving me mad looking at and updating code that doesn't do it
Perhaps a refactor is in order? The senior engineer would learn from it and it would reduce your angst. Double win.
wizardzz wrote:
All my event handlers call functions, nothing more.
Excellent. My WinForms event handlers call methods in the
Form's view model. /raviMy new year resolution: 2048 x 1536 Home | Articles | My .NET bits | Freeware ravib(at)ravib(dot)com
-
Not a question, more of a survey. How many people here always separate code from event handlers, especially Form events. I always do it, but it is driving me mad looking at and updating code that doesn't do it, written by a senior engineer. All my event handlers call functions, nothing more.
Craigslist Troll: litaly@comcast.net "I have a theory that the truth is never told during the nine-to-five hours. " — Hunter S. Thompson
Always always always... Well - sometimes while developing and testing I don't immediately - but I always try and move code out of the handler because otherwise some goose will come along and call the handler in order to execute the code
MVVM# - See how I did MVVM my way ___________________________________________ Man, you're a god. - walterhevedeich 26/05/2011 .\\axxx (That's an 'M')
-
Not a question, more of a survey. How many people here always separate code from event handlers, especially Form events. I always do it, but it is driving me mad looking at and updating code that doesn't do it, written by a senior engineer. All my event handlers call functions, nothing more.
Craigslist Troll: litaly@comcast.net "I have a theory that the truth is never told during the nine-to-five hours. " — Hunter S. Thompson
I use the MVP pattern and forms or user controls as views. The views are held 'dumb', meaning that they barely 'know' how to show the data sent by the presenter. Event handlers don't do very much more than pass input on to the presenter, but that's just another variant of wrapping up all logic in methods.
"I just exchanged opinions with my boss. I went in with mine and came out with his." - me, 2011 ---
I am endeavoring, Madam, to construct a mnemonic memory circuit using stone knives and bearskins - Mr. Spock 1935 and me 2011 -
I pass
nullinstead.I doubt you can. You can pass null for the sender, but not the eventargs, I don't think.
Christian Graus Driven to the arms of OSX by Vista. Read my blog to find out how I've worked around bugs in Microsoft tools and frameworks.
-
Not a question, more of a survey. How many people here always separate code from event handlers, especially Form events. I always do it, but it is driving me mad looking at and updating code that doesn't do it, written by a senior engineer. All my event handlers call functions, nothing more.
Craigslist Troll: litaly@comcast.net "I have a theory that the truth is never told during the nine-to-five hours. " — Hunter S. Thompson
I don't even have event handlers, I inline my event handling code in an anonymous delegate ;P
-
Not a question, more of a survey. How many people here always separate code from event handlers, especially Form events. I always do it, but it is driving me mad looking at and updating code that doesn't do it, written by a senior engineer. All my event handlers call functions, nothing more.
Craigslist Troll: litaly@comcast.net "I have a theory that the truth is never told during the nine-to-five hours. " — Hunter S. Thompson
Mostly the same, here, it may involve a condition check such as
(!m_bLalalalaIcannotHearYou)or determining parameters for the call.
What do you think of this?
this.listBox1.DoubleClick +=(x, y) => m_ipl.BeginEdit();
Just discovered that code (written a few month agon, rotting away...)
FILETIME to time_t
| FoldWithUs! | sighist | WhoIncludes - Analyzing C++ include file hierarchy -
I'll second JSOP's method. Why cause function call overhead if you don't need to? Probably a moot point, especially if it's a button click, but something like a resize or repaint event could be called thousands of times.
- S 50 cups of coffee and you know it's on! Code, follow, or get out of the way.
Because you may need to trigger the action differently - e.g. from different events, or programmatically. Now, you might say "I will isolate it when I need it" - but the issue goes deeper in my experience. When you put the code immediately in the handler, you can make all kinds of assumptions: The user just clicked that button, so the form is probably on screen and active and the user didn't do anything else in the last 10 ms. These implicit assumptions pile up over time, and make maintenance hard. If you put the action into a public method right away you have to make sure the method can be called anytime by anyone (or understand and document trhe cases where this isn't). Separate the what from the when - this does more for UI/functionality independence than putting it into separate classes. Now yes, that's overhead. However, in my experience the "quick and dirty" method leads often leads to repeated operations in the long run because it's impossible to detangle the call paths of the helepr method. My personal warning sign is methods getting parameters like "doUpdate", "refresh", "updateImmediately" etc. are a warning sign for that.
FILETIME to time_t
| FoldWithUs! | sighist | WhoIncludes - Analyzing C++ include file hierarchy -
I had an old school teacher that that concept was one of the first things he taught us. 1. put it in a method that can be called as a general rule 2. name the method something useful and meaningful. 3. add comments where necessary to clear things up further. The inherited code I'm looking at right now does pretty much none of the above. well over 100,000 lines of code and I have found 5 comments, one of which i paraphrase a tad bit "the order of these items must match the order of the input or bad things will happen" That is the most informative comment in the entire dang thing. and he liked names such as
public static string Code()Programming is a race between programmers trying to build bigger and better idiot proof programs, and the universe trying to build bigger and better idiots, so far... the universe is winning. A graduation ceremony is an event where the commencement speaker tells thousands of students dressed in identical caps and gowns that 'individuality' is the key to success
gavindon wrote:
he liked names such as
public static string Code()I'm not surprised, I find I use the first three a lot too. The last one I don't use myself, but that's just one of my foibles
Graham Librarians rule, Ook!
-
I doubt you can. You can pass null for the sender, but not the eventargs, I don't think.
Christian Graus Driven to the arms of OSX by Vista. Read my blog to find out how I've worked around bugs in Microsoft tools and frameworks.
I pass
nullfor both; have for years. -
I can understand one or two lines, easily. I can't handle the mammoth 20-100 lines in an event handler. I don't know if I'm being idiosyncratic, but it drives me mad!
Craigslist Troll: litaly@comcast.net "I have a theory that the truth is never told during the nine-to-five hours. " — Hunter S. Thompson
wizardzz wrote:
I can understand one or two lines, easily. I can't handle the mammoth 20-100 lines in an event handler. I don't know if I'm being idiosyncratic, but it drives me mad!
I love these kind of questions. I suspect the kernel of this issue is something to do with the notion of separating UI from logic. It's sound thinking. There are few things worse than a mess of code implemented directly behind the controls on a form. So...Is it "good practice" to move code out of event handlers and provide simple calls from the handler to the extracted method? I say no. But stay with me, and don't throw that tomato yet... An event handler is just a method. The fact that it's not "called" by your code in the traditional sense doesn't make it any less of a method. Same rules for determining the quality of a method should apply. If the quality of the code in the event handler is good, then there's generally little to be gained by moving it, except another layer of indirection. If the quality of the code is bad then moving it out without refactoring it won't make it any better. So, to answer your first question, NO, I wouldn't simply call out from an event handler to another method. An event handler should do it's job...handle an event. Which ties in with your idea that you are ok with a few lines of code, but big mammoth event handlers are the real problem. Absolutely, but it's the bigness and the mammothness of them that's the problem. Not the Eventness of them. The key as with any problem method is to clean up the code. Decompose those 100 lines into perhaps 4 or 5 methods, or more. Whether you then orchestrate calls to those methods directly from the event handler, or from some other method that the handler calls is up to you. Here's a question. When you extract the code from an event handler in another method, where does that method go? If it's just another private method in the Form or WebForm then you haven't really achieved much. If on the other hand you devise a sort of service method separate from the UI, then extracting the code from the event handler works well. The key as I've said is that you have to tidy up the code, not simply move it out of the event handler for the sake of moving it. Incidently moving code out of the UI into a service layer also makes it easier to Test. Another good reason to do it. It's not the almost empty event handlers that we're aiming for, it's the set of well defined
-
I doubt you can. You can pass null for the sender, but not the eventargs, I don't think.
Christian Graus Driven to the arms of OSX by Vista. Read my blog to find out how I've worked around bugs in Microsoft tools and frameworks.
It's only a problem if the recipient is expecting a value and doesn't do a null check. ~99% of events that take the base EventArgs don't look at it, so it's generally safe. IIRC my code is 100% for not looking at the useless thing.
3x12=36 2x12=24 1x12=12 0x12=18
-
wizardzz wrote:
I can understand one or two lines, easily. I can't handle the mammoth 20-100 lines in an event handler. I don't know if I'm being idiosyncratic, but it drives me mad!
I love these kind of questions. I suspect the kernel of this issue is something to do with the notion of separating UI from logic. It's sound thinking. There are few things worse than a mess of code implemented directly behind the controls on a form. So...Is it "good practice" to move code out of event handlers and provide simple calls from the handler to the extracted method? I say no. But stay with me, and don't throw that tomato yet... An event handler is just a method. The fact that it's not "called" by your code in the traditional sense doesn't make it any less of a method. Same rules for determining the quality of a method should apply. If the quality of the code in the event handler is good, then there's generally little to be gained by moving it, except another layer of indirection. If the quality of the code is bad then moving it out without refactoring it won't make it any better. So, to answer your first question, NO, I wouldn't simply call out from an event handler to another method. An event handler should do it's job...handle an event. Which ties in with your idea that you are ok with a few lines of code, but big mammoth event handlers are the real problem. Absolutely, but it's the bigness and the mammothness of them that's the problem. Not the Eventness of them. The key as with any problem method is to clean up the code. Decompose those 100 lines into perhaps 4 or 5 methods, or more. Whether you then orchestrate calls to those methods directly from the event handler, or from some other method that the handler calls is up to you. Here's a question. When you extract the code from an event handler in another method, where does that method go? If it's just another private method in the Form or WebForm then you haven't really achieved much. If on the other hand you devise a sort of service method separate from the UI, then extracting the code from the event handler works well. The key as I've said is that you have to tidy up the code, not simply move it out of the event handler for the sake of moving it. Incidently moving code out of the UI into a service layer also makes it easier to Test. Another good reason to do it. It's not the almost empty event handlers that we're aiming for, it's the set of well defined
After reading through many replies to my original post, I have determined that a lot of my frustration stems from the lack of any documentation in any code, along with poorly and inconsistently named variables. This combination with event handlers containing what should be thrown into a nicely callable method, has made deciphering what any control actually does nearly impossible.
Craigslist Troll: litaly@comcast.net "I have a theory that the truth is never told during the nine-to-five hours. " — Hunter S. Thompson
-
Because you may need to trigger the action differently - e.g. from different events, or programmatically. Now, you might say "I will isolate it when I need it" - but the issue goes deeper in my experience. When you put the code immediately in the handler, you can make all kinds of assumptions: The user just clicked that button, so the form is probably on screen and active and the user didn't do anything else in the last 10 ms. These implicit assumptions pile up over time, and make maintenance hard. If you put the action into a public method right away you have to make sure the method can be called anytime by anyone (or understand and document trhe cases where this isn't). Separate the what from the when - this does more for UI/functionality independence than putting it into separate classes. Now yes, that's overhead. However, in my experience the "quick and dirty" method leads often leads to repeated operations in the long run because it's impossible to detangle the call paths of the helepr method. My personal warning sign is methods getting parameters like "doUpdate", "refresh", "updateImmediately" etc. are a warning sign for that.
FILETIME to time_t
| FoldWithUs! | sighist | WhoIncludes - Analyzing C++ include file hierarchyCompletely agree with you. I actually tend to break things up into separate into functions, except for ultra simple ui stuff. This just comes with experience and recognizing/anticipating patterns. I was just pointing out a reason (performance) not to do this, which with today's super computers on our desks, it's not really an issue anymore.
- S 50 cups of coffee and you know it's on! Code, follow, or get out of the way.
-
Because you may need to trigger the action differently - e.g. from different events, or programmatically. Now, you might say "I will isolate it when I need it" - but the issue goes deeper in my experience. When you put the code immediately in the handler, you can make all kinds of assumptions: The user just clicked that button, so the form is probably on screen and active and the user didn't do anything else in the last 10 ms. These implicit assumptions pile up over time, and make maintenance hard. If you put the action into a public method right away you have to make sure the method can be called anytime by anyone (or understand and document trhe cases where this isn't). Separate the what from the when - this does more for UI/functionality independence than putting it into separate classes. Now yes, that's overhead. However, in my experience the "quick and dirty" method leads often leads to repeated operations in the long run because it's impossible to detangle the call paths of the helepr method. My personal warning sign is methods getting parameters like "doUpdate", "refresh", "updateImmediately" etc. are a warning sign for that.
FILETIME to time_t
| FoldWithUs! | sighist | WhoIncludes - Analyzing C++ include file hierarchypeterchen wrote:
Now, you might say "I will isolate it when I need it" - but the issue goes deeper in my experience. When you put the code immediately in the handler, you can make all kinds of assumptions: The user just clicked that button, so the form is probably on screen and active and the user didn't do anything else in the last 10 ms. These implicit assumptions pile up over time, and make maintenance hard.
If you put the action into a public method right away you have to make sure the method can be called anytime by anyone (or understand and document trhe cases where this isn't). Separate the what from the when - this does more for UI/functionality independence than putting it into separate classes.I'm skeptical about this. Knowing that
DooFoo()is only called from theFooButtonClicked()event handler, what makes you think that a code monkey won't just xfer his full set of assumptions to the method that came from the event handler?3x12=36 2x12=24 1x12=12 0x12=18
