views:

333

answers:

5

Which is the preferred/recommended way of handling events in .NET:

this.Load += new EventHandler(Form1_Load);
private void Form1_Load(object sender, EventArgs e)
{ }

or

protected override void OnLoad(EventArgs e)
{
    base.OnLoad(e);
}

What would the pros/cons of each method be? I've used both methods over the years, and have usually leaned more towards the first method simply because that is what Visual Studio creates automatically for handling events. Are there any benefits to the second method that I am missing though?

A: 

The overridden method is preferrable as it will be invoked polymorphically virtually by the CLR.

[Edit] Here is why I believe the overridden method is preferable:

Here is a simple example:

class Foo
{
    public event EventHandler Changed = delegate { };

    protected virtual void OnChanged()
    {
     this.Changed(this, EventArgs.Empty);
    }
}

class Bar : Foo
{
    public Bar()
    {
     this.Changed += new EventHandler(this.Bar_Changed);
    }

    void Bar_Changed(Object sender, EventArgs e) { }
}

class Baz : Foo
{
    protected override void OnChanged() 
    { 
        base.OnChanged();
    }
}

Now I believe the Baz is the better implementation and here is why. Bar must do the following IL instructions to wire up the event:

    L_000a: ldftn instance void Bar::Bar_Changed(object, class [mscorlib]System.EventArgs)
    L_0010: newobj instance void [mscorlib]System.EventHandler::.ctor(object, native int)
    L_0015: call instance void Foo::add_Changed(class [mscorlib]System.EventHandler)

We must create a delegate to the handling method, an instance of the EventHandler and then call the add_Changed method on the event in the base class. While these are not performance killers none of the previous code is required for Baz to work. Since any call to OnChanged will be virtual the only performance penalty will be the CLR finding the right instance method to call in the inheritance chain.

Andrew Hare
Can you explain this a bit further - I'm not sure I understand what you mean.
Jon Tackabury
+4  A: 

It depends entirely on where you want to catch the event and why.

The first method (wire-up) is for when you want some other class to handle the event. And you may need to do that for a number of reasons; the other class may have access to services that perform some complex logic or whatever. The point is that you use the first method when you want a separate observer to respond to the event.

The second method (overriding) is for when you want the form to respond because it can; because it's responsibility is local.

Chris Holmes
Good point, I forgot about something else handling an event. In the case of the form handling it's own event, why does VS wire up event handlers instead of using the protected overrides?
Jon Tackabury
You'd have to ask MS about that decision. I don't honestly know.
Chris Holmes
+6  A: 

The first way is what Microsoft suggests. The pattern is:

  1. some code wants to raise an event, calls OnXxx
  2. OnXxx makes a call to the delegate
  3. Wired event handlers are called

If you perform the second model, you risk forgetting the base.OnXxx call and breaking everything. Nice part of option 2 is that you get to decide if you are called before or after all of the other event handlers. If you put your code before the base.OnXxx, you get executed before the event does. Of course the first model can be used always, the second only if you are subclassing the class raising the event.

Teun D
+1  A: 

Though not the original question, I want to point out that:

this.Load += new EventHandler(Form1_Load);

can be written as:

this.Load += Form1_Load;

The delegate construction is inferred.

Jay Bazuzi
Load += Form1_Load; // :)
DK
@DK: A matter of style. I prefer to have 'this.' but many do not, and it's fine.
Jay Bazuzi
A: 

There is no hard & fast rule but there are gotchas with both the approaches. You select one you can avoid easily.

Delegates

lot of developers place the += code in a place where it can get called repeatedly. At least lot of novices do that. As a result there will be 'n' entries in the delegate list maintained by the owner control and all of them get called. Simple way to avoid is to place the += calls in constructor kind of place which get called only once.

OnXXXX

The risk is with forgetting to call the base.XXX methods. This has been a common source of bugs and most Windows programmers are aware of the problems if you miss to call the base class versions - this is particular for the case of Windows messages (paint etc).

Sesh