Calling functions from Events
-
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
-
peterchen 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
Thanks for your feedback.
Dan Neely wrote:
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
Nothing. For me it's just a practice that makes me think about a clean interface, separating functionality from the event, and it documents that separation in the code. That last point makes it "not just personal" for me. Of course you can have an
EnableEditMode()method that fails spectacularly whenbutton2is disabled. And I can have anEnableEditModeButOnlyWhenButton2IsEnabledmethod. Both sucks - but in both cases, I would argue the code is better even without any comments: In the first case, the code shows clearly that when Event X fires, EditMode should be enabled. The function is well named, but has a bug. In the second case, the interface already "smells" wrong.
Now, this may be an idealized example, and I don't think it's necessary to do this separation, just good practice. Code is never perfect, we usually can't even tell if it's good enough, but we can tell better from worse.
FILETIME to time_t
| FoldWithUs! | sighist | WhoIncludes - Analyzing C++ include file hierarchy -
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
That's what I say all the time. Take some time and document the damn method. Let me in your "beautiful" mind and enlighten me with this method's purpose so i don't have to spend hours extracting it from the bare code. I have seen this project exquisitely architected, exuding of design patterns and decoupling all over the place. Very manageable they say. Still, lacking documentation in 90% of the cases, made it so hard to understand for new people that soon on top of that beautifully crafted architecture people started to throw patches and "Band-Aid" code. So, what good is it that you decoupled stuff, when it made it so abstract, that new people have a hard time understanding it, ‘cause you did not document. Doesn't that bring us back to code management trouble? I don't know man, maybe it's just me. Cheers.
giuchici
-
Same here. If it's a couple of lines, it stays in the event handler. Otherwise, it's going into it's own method, even if, right now, it's only called by the event handler. You never know. Tomorrow, you'll find out that you now need to call it from multiple places. But, the biggest reason I break it out into its own method because of "self documenting code". Why this is such a weird concept no-a-days is beyond me. Put the code into a method and name the method something that describes what the code does! OH MY GOD! WHAT A CONCEPT!! Imagine making debugging easier! ;)
A guide to posting questions on CodeProject[^]
Dave Kreskowiak -
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
Regarding comments: It is best to summarize right at the beginning of a method what it is supposed to do. You then write the code in a clear and self-commenting manner that makes it easy for you and other programmers to see how it is being done. So if it turns out that there are times when the result of the method is inconsistent with the summary, it is easier to debug. The other benefit of doing it this way is that the developer has given thought to the method's objective before actually writing the code that does it.
-
Same here. If it's a couple of lines, it stays in the event handler. Otherwise, it's going into it's own method, even if, right now, it's only called by the event handler. You never know. Tomorrow, you'll find out that you now need to call it from multiple places. But, the biggest reason I break it out into its own method because of "self documenting code". Why this is such a weird concept no-a-days is beyond me. Put the code into a method and name the method something that describes what the code does! OH MY GOD! WHAT A CONCEPT!! Imagine making debugging easier! ;)
A guide to posting questions on CodeProject[^]
Dave KreskowiakWell, since Event Handlers can be named as well, you could create self-documenting code without necessarily having to create a separate method. Of course, if your event handler is button18_Click that isn't going to work.
-
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 is an anti-pattern defined for what you described: Magic Pushbutton[^]. Another consideration is whether to handle the event asynchronously or not. In most cases I handle the event in a different thread and disable the button (or other control) until the event handling code finishes. That way the UI doesn't become unresponsive while waiting. Of course this is uneccessary for very short operations.
-
Well, since Event Handlers can be named as well, you could create self-documenting code without necessarily having to create a separate method. Of course, if your event handler is button18_Click that isn't going to work.
Part of self-documenting code is the parameters that are passed into the method. Sure, you could name a Button.Click event handler something more useful, like GetSccmFolders, but what does the
sender (Object)andSystemEventArgsor even theHandlesclause have to do with getting Sccm Folders? Nothing! So why is it there? This just leads to misleading documentation and code that has to be called in very specific ways that have nothing to do with the operation the method is really performing.A guide to posting questions on CodeProject[^]
Dave Kreskowiak -
Part of self-documenting code is the parameters that are passed into the method. Sure, you could name a Button.Click event handler something more useful, like GetSccmFolders, but what does the
sender (Object)andSystemEventArgsor even theHandlesclause have to do with getting Sccm Folders? Nothing! So why is it there? This just leads to misleading documentation and code that has to be called in very specific ways that have nothing to do with the operation the method is really performing.A guide to posting questions on CodeProject[^]
Dave Kreskowiak -
Same here. If it's a couple of lines, it stays in the event handler. Otherwise, it's going into it's own method, even if, right now, it's only called by the event handler. You never know. Tomorrow, you'll find out that you now need to call it from multiple places. But, the biggest reason I break it out into its own method because of "self documenting code". Why this is such a weird concept no-a-days is beyond me. Put the code into a method and name the method something that describes what the code does! OH MY GOD! WHAT A CONCEPT!! Imagine making debugging easier! ;)
A guide to posting questions on CodeProject[^]
Dave KreskowiakDave Kreskowiak wrote:
You never know. Tomorrow, you'll find out that you now need to call it from multiple places.
The biggest reason for separate methods, in my experience. I don't work for MS, but one thing they get right (imo) is to have multiple ways of doing almost anything. That is easiest to handle with a single well-named and well-designed method (or "umbrella" method) called by multiple event handlers. This way, I can be reasonably sure that every UI action that enters that code will behave the same way.
-
I agree with you. But what does GetSccmFolders() mean? In other words, is it a good example of self-documenting code?
No, it's not a good example of self-documenting code, but it's also not what you would call an event handler either. The point of the post wasn't so much what you called the method, but that the arguments don't match what the method does.
A guide to posting questions on CodeProject[^]
Dave Kreskowiak -
Overall it is a good idea to call functions, that way you can change the logic pretty easily and also make the code readable (assuming functions have good names). It is pretty easy these days with refactoring support to extract the code into separate methods. For instance,
void serverName_changed(object sender, EventArgs e)
{
ResetUserNameAndPassword();
}is more readable then
void serverName_changed(object sender, EventArgs e)
{
Username.Text = "";
Password.Text = ""
// Some other code usually gets very long
//}
you still end up with:
void serverName_changed(object sender, EventArgs e)
{
ResetUserNameAndPassword();
}
...
void ResetUserNameAndPassword(void)
{
Username.Text = "";
Password.Text = ""
// Some other code usually gets very long
//
}The only thing you have really done is add an additional call to the execution stream. Granted, if the event is not a critical timed event handler, it probably won't matter. But if the event is generated 100s or 1000s of times a second, the extra code could have an impact on performance.
Steve _________________ I C(++) therefore I am
-
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 almost always call functions from events, but if I need to act on the control I put all that logic in the event (e.g. RunButton_Click() will change the text to "Stop") Ionce had an engineer working for me who liked to raise control events to use the functionality he put in the event handlers. Lots of very interesting bugs crawled out of that practice. He never quite caught on to the problem.
-
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 typically guard condition, delegate to some other logic outside of an event model, and then bubble if applicable. I also try to reuse the same abstracted event as much as possible and deal w/ variability via arguments vs having an event per usage. I feel that event handlers are just listeners / hooks and not workers. Just middle men basically, that facilitate decoupled collaboration. I do make some exceptions where common usage dictates...like a repeater item data bound event handler; it doesn't always make sense to punt the binding if it's tiny. But for non-trivial scenarios I'll push the repeater's body template into a separate component with a "Bind(someModel)" method, and forward to it from the event handler on the containing page or component. As it happens, I'm currently refactoring a ASP.NET client server app towards patterns, paying back a big design debt incurred by the original developers. Refactoring bloated event handlers in this app is one of the biggest sources of code improvement. In particular I'm frequently finding baskin robbins 31 flavor events that all basically do the same thing with only slight variations. This one is chocolate, that one is double fudge, that one is choc-vanilla swirl, etc. Extracting the commonality and variabilizing the differences has significant impact on thinning down code behinds and standardizing the logic; it also has lead to the detection of latent or inconsistent bugs, and generally been a major enhancement to the code quality. It's also a lot easier to read / maintain the end product.
-
you still end up with:
void serverName_changed(object sender, EventArgs e)
{
ResetUserNameAndPassword();
}
...
void ResetUserNameAndPassword(void)
{
Username.Text = "";
Password.Text = ""
// Some other code usually gets very long
//
}The only thing you have really done is add an additional call to the execution stream. Granted, if the event is not a critical timed event handler, it probably won't matter. But if the event is generated 100s or 1000s of times a second, the extra code could have an impact on performance.
Steve _________________ I C(++) therefore I am
Steve Mayfield wrote:
. But if the event is generated 100s or 1000s of times a second, the extra code could have an impact on performance.
If a UI event handler gets called that many times, you have other issues to be sorted out. In practice it does not happen that much. Also remember the JIT optimizer. It has a role to play too. I am always for making the code readable and worry about performance later. These minor things usually do not cause performance issue. At least not in my career so far.
-
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
We have a policy for our team: Event handlers should (almost) always be trivial implementations, and delegate anything non-trivial to a different method and/or thread. This isn't just about separating UI from business logic, although that in itself is a good enough argument. Any code, not just UI, can raise events. It's a great way of dynamically coupling systems together. The risk in this is that the sender has no idea about who might subscribe to the event in the future, or what they may need to do. How can you design the sending code when you don't know (a) how long it will take before the event handlers return, (b) what the event handlers will do, (c) what program state they might change, (d) what calls they might make back to the sender to get a bit more information, (e) what threading implications there may be, (f) what exceptions it might throw?? Imagine you raise an event on a worker thread. Some UI code subscribes to this event. It needs to update a button, so it Invokes across to the UI thread. The worker may now be blocked for a long time (so performance could suffer) or possibly even deadlocked by the UI thread. This is very difficult to design for unless you follow some best-practices in your event handlers. Another common problem is when a system responds to an event, and it changes its own state. When its state is changed, it of course raises its own events to allow other systems to react to those specific changes. Pretty soon we wind up with a cascading event that crushes performance, or possibly even reentrancy problems. An issue that often occurs when multiple listeners subscribe to an event is that it is hard to control the order in which the even handlers are called, so they can begin to have side effects on each other (and worse, the side effects change if the order of calls changes). A way of mitigating this is to minimise the work done in event handlers - the more complex they are the higher the risks. Another common problem with events is that if we react to them immediately, we can end up doing a lot of unnecessary repetitive work. Imagine if we raise an event every time our document changes, and the UI reacts to this to redraw a window every time the event fires. Seems logical. But then we load the document from disk and as the structure is built piece by piece, the event is fired 300 times, causing 300 redraws of the window (result: extremely slow and flickery loading). So then you add a bodge to suppress those updates during loading. And then you find a similar prob
-
Naerling wrote:
The only thing that is not happy is my software, because it cannot feel emotions
I wish I could upset or frustrate my software sometimes, then it might have some empathy and stop doing the same to me!
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:
and stop doing the same to me!
It's supposed to do the same thing to you, isn't it? :laugh: Part of what happens in the Event is that a lot of information is passed, splitting it up and using only the parts you need elsewhere can avoid confusion about what the function is doing.
-
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
Yip. Welcome to life as a programmer. There's a really interesting Goldilocks issue with code. If it's too messy and tangled, it's a pain to work with. If it's too abstracted and over-engineered, it's a pain to work with. Somewhere in the middle there's code that does just what it needs to do in a direct well factored way, no tangled mess of cut and paste login, but also no excessive abstraction to the point where the point is lost. Such code is hard to find, but I'm hopeful that as we move to a point where "testability" is viewed as more and more important, we'll see more and more maintainable code. I'm also hopeful that as more and more developers get involved in Open Source projects earlier in their career (ideally while still in college) they'll emerge into the world as better developers, and the craft of Software Development will get handed on in a more direct way than it has in the past. Formal education sure as hell isn't up to the job. -Rd
Hit any user to continue.
-
We have a policy for our team: Event handlers should (almost) always be trivial implementations, and delegate anything non-trivial to a different method and/or thread. This isn't just about separating UI from business logic, although that in itself is a good enough argument. Any code, not just UI, can raise events. It's a great way of dynamically coupling systems together. The risk in this is that the sender has no idea about who might subscribe to the event in the future, or what they may need to do. How can you design the sending code when you don't know (a) how long it will take before the event handlers return, (b) what the event handlers will do, (c) what program state they might change, (d) what calls they might make back to the sender to get a bit more information, (e) what threading implications there may be, (f) what exceptions it might throw?? Imagine you raise an event on a worker thread. Some UI code subscribes to this event. It needs to update a button, so it Invokes across to the UI thread. The worker may now be blocked for a long time (so performance could suffer) or possibly even deadlocked by the UI thread. This is very difficult to design for unless you follow some best-practices in your event handlers. Another common problem is when a system responds to an event, and it changes its own state. When its state is changed, it of course raises its own events to allow other systems to react to those specific changes. Pretty soon we wind up with a cascading event that crushes performance, or possibly even reentrancy problems. An issue that often occurs when multiple listeners subscribe to an event is that it is hard to control the order in which the even handlers are called, so they can begin to have side effects on each other (and worse, the side effects change if the order of calls changes). A way of mitigating this is to minimise the work done in event handlers - the more complex they are the higher the risks. Another common problem with events is that if we react to them immediately, we can end up doing a lot of unnecessary repetitive work. Imagine if we raise an event every time our document changes, and the UI reacts to this to redraw a window every time the event fires. Seems logical. But then we load the document from disk and as the structure is built piece by piece, the event is fired 300 times, causing 300 redraws of the window (result: extremely slow and flickery loading). So then you add a bodge to suppress those updates during loading. And then you find a similar prob
Damn good advice. I've thought through some of your points before, and I think that's why I developed my preference to separate events and complex logic. In particular this scenario:
Zot Williams wrote:
Another awful approach I've seen is to write a complex method in an event handler, and then call that event-handling method directly to "internally trigger" the code, usually passing in dummy sender/args values.
Seriously, thank you for putting all of this into a great post!
Craigslist Troll: litaly@comcast.net "I have a theory that the truth is never told during the nine-to-five hours. " — Hunter S. Thompson
-
Yip. Welcome to life as a programmer. There's a really interesting Goldilocks issue with code. If it's too messy and tangled, it's a pain to work with. If it's too abstracted and over-engineered, it's a pain to work with. Somewhere in the middle there's code that does just what it needs to do in a direct well factored way, no tangled mess of cut and paste login, but also no excessive abstraction to the point where the point is lost. Such code is hard to find, but I'm hopeful that as we move to a point where "testability" is viewed as more and more important, we'll see more and more maintainable code. I'm also hopeful that as more and more developers get involved in Open Source projects earlier in their career (ideally while still in college) they'll emerge into the world as better developers, and the craft of Software Development will get handed on in a more direct way than it has in the past. Formal education sure as hell isn't up to the job. -Rd
Hit any user to continue.
Richard A. Dalton wrote:
I'm also hopeful that as more and more developers get involved in Open Source projects earlier in their career (ideally while still in college) they'll emerge into the world as better developers, and the craft of Software Development will get handed on in a more direct way than it has in the past.
Formal education sure as hell isn't up to the job.I had a lengthy conversation with Ms. wizardzz last night on the same topic. She works in PHP for a living now. We both felt that formal education in the industry often does very little to show how coding is done in the "real world." I won't bore you with the details, but it was pretty much the same feelings as your post! Amazing, are you eavesdropping on us?
Craigslist Troll: litaly@comcast.net "I have a theory that the truth is never told during the nine-to-five hours. " — Hunter S. Thompson