tags:

views:

171

answers:

4

Image a Button on your windows form that does something when being clicked.

The click events thats raised is typically bound to a method such as

protected void Button1_Click(object sender, EventArgs e) {

}

What I see sometimes in other peoples' code is that the implementation of the buttons' behaviour is not put into the Button1_Click method but into an own method that is called from here like so:

private DoStuff() { }

protected void Button1_Click(object sender, EventArgs e) { this.DoStuff(); }

Although I see the advantage here (for instance if this piece of code is needed internally somewhere else, it can be easily used), I am wondering, if this is a general good design decision?

So the question is: Is it a generally good idea to put event handling code into an own method and if so what naming convention for those methods are proven to be best practice?

+1  A: 

A good rule of thumb is to see whether the method is doing sometihng UI specific, or actually implementing a generic action. For example a buttock click method could either handle a click or submit a form. If its the former kind, its ok to leave it in the button_click handler, but the latter deserves a method of its own. All I'm saying is, keep the single responsibility principle in mind.

Pramod
Thanks for the reference to the principle. Although it is a pretty natural way of thinking about that, it is good to know that other people already thought a lot about it and even gave it a name. ;-)
Mephisztoe
+2  A: 

I put the event handling code into a separate method if:

  • The code is to be called by multiple events or from anywhere else or
  • The code does not actually have to do with the GUI and is more like back-end work.

Everything small and only GUI-related goes always into the handler, sometimes even if it is being called from the same event (as long as the signature is the same). So it's more like, use a separate method if it is a general action, and don't if the method is closely related to the actual event.

OregonGhost
Just as with Pramod's answer this is a good explanation for a good strategy. Thanks!
Mephisztoe
A: 

The straight answer is yes. It's best to encapsulate the task into it's own method not just for reuse elsewhere but from an OOP perspective it makes sense to do so. In this particular case clicking the button starts a process call DoStuff. The button is just triggering the process. Button1_Click is a completely separate task than DoStuff. However DoStuff is not only more descriptive, but it decouples your button from the actual work being done.

A good rule of thumb is to see whether the method is doing sometihng UI specific, or actually implementing a generic action. For example a buttock click method could either handle a click or submit a form. If its the former kind, its ok to leave it in the button_click handler, but the latter deserves a method of its own. All I'm saying is, keep the single responsibility principle in mind.

I put the event handling code into a separate method if:

  • The code is to be called by multiple events or from anywhere else or

  • The code does not actually have to do with the GUI and is more like back-end work.

Everything small and only GUI-related goes always into the handler, sometimes even if it is being called from the same event (as long as the signature is the same). So it's more like, use a separate method if it is a general action, and don't if the method is closely related to the actual event.

Micah
A: 

In this case, I'd keep the DoStuff() method but subscribe to it with:

button1.Click += delegate { DoStuff(); }

Admittedly that won't please those who do all their event wiring in the designer... but I find it simpler to inspect, personally. (I also hate the autogenerated event handler names...)

Jon Skeet
Yeah, well, subjective I guess, but if you're working with anyone who 'does' use the desginer, they're going to be expecting the 'standard' naming convention (I don't see any problem with it).
Benjol
The problem with the designer-generated names are that they indicate where the event came from rather than what the handler does. This is contrary to normal naming. (And they contain underscores, of course...)
Jon Skeet