.NET: Is creating new EventArgs every time the event fires a good practice?

Question

For example, I have a base event publishing method:

    protected virtual OnSomeEvent(EventArgs e)
    {
        var handler = SomeEvent;
        if (handler != null)
        {
            handler(this, e);
            // handler(this, new EventArgs());// EDIT: Yes it should be
                                           // handler(this, e),
                                           // ignore this one :D
        }
    }

For a derived class that overrides OnSomeEvent and raises an additional event when it fires:

    protected override OnSomeEvent(EventArgs e)
    {
        base.OnSomeEvent(e);

        if (ExtendedEvent != null)
        {
            OnExtendedEvent(e);
        }
    }

    protected void OnExtendedEvent(EventArgs e)
    {
       // some stuff done
       // new information the ExtendedEventArgs object needs 
       //  is not available until this point

       ExtendedEvent(this, new ExtendedEventArgs(someStuff, someOtherStuff));
    }

And if derivation goes on like this, it will create a new derived EventArgs for each generation of derived class that requires it. However it seems various derivations of EventArgs on the .NET framework are not designed to be mutable (no setters), this discourages an object from keeping a single instance of EventArgs and modify it as it goes.

So every time an event like this fires, it will re-allocate memory for all involved EventArgs objects. In a graphic intense application where an event can be triggered dozens of times per second (such as OnPaint event on a control), is this really a good practice?

Should I make some changes to OnExtendedEvent() and make ExtendedEventArgs mutable so the following is possible?

    protected ExtendedEventArgs extendedArgs = ExtendedEventArgs.Empty;
    protected void OnExtendedEvent(EventArgs e)
    {
       // some stuff done
       // new information the ExtendedEventArgs object needs 
       //  is not available until this point

       extendedArgs.someProperty1 = someStuff;
       extendedArgs.someProperty2 = someOtherStuff;

       ExtendedEvent(this, extendedArgs);
    }

EDIT: Fixed the example code, should be clearer now.

Djokjakarta

Derivation is not a problem, the derived class can only call base.OnExtendedEvent with EventArgs.Empty. In general, don't be afraid to create short-lived garbage. – Bradshaw

I think you mean immutable instead of mutable. Immutable means you cannot change it. – Hight

Answer 1

I would create a new immutable object each time it is fired, as there are values in the event arguments.

The main reason is the what would happen if a new event is fired again while an existing event is being handled?

This will possibly happen in multi-threaded applications but may even happen on a single thread as shown by the following example:

First event is fired with the following values:

extendedArgs.someProperty1 = "Fire 1";
extendedArgs.someProperty2 = "Fire 1 Other Stuff";

Then somehow the first event handler does something causes the event to be fired again with the following arguments:

extendedArgs.someProperty1 = "Fire 2";
extendedArgs.someProperty2 = "Fire 2 Other Stuff";

All the event handlers are for the second event are processed, and now we are back to processing the rest of the event handlers for the first event.

Now since the same object is used all the event handlers for the first event will now be have "Fire 2" as their someProperty1, as the second event overwrote the values.

As @nobugz mentioned don't be afraid to create short-lived garbage.

Hight

Answer 2

First off, why take an EventArgs argument to your firing method if you are just ignoring it anyway? That is the real waste, but the resource consumption is less problematic than the lie that your method is telling its callers. Just pass the argument on through, your firing method likely will not have relevant info accessible to create the EventArgs object anyway:

protected virtual OnSomeEvent(EventArgs e)
{
    var handler = SomeEvent;
    if (handler != null)
    {
        handler(this, e);
    }
}

So, now that we have that straight, if your EventArgs object has no meaningful information to tell your subscribers, just use EventArgs.Empty, that's what it is there for. You could follow the same pattern for your custom EventArgs classes, but honestly, you are worrying about nothing. Creating EventArgs objects will never be a bottleneck in your application, and if it is, you have design problems.

Marshall

Answer 3

I'm a little confused by your OnExtendedEvent code - are you meaning to redispatch the event as a SomeEvent?

When a client adds an event handler, they expect they are able to remove the event handler while handling the event, like this:

someObject.SomeEvent += OnSomeEvent;
// ...
private void OnSomeEvent(object sender, EventArgs e)
{
    someObject.SomeEvent -= OnSomeEvent;
}

If you do not follow the standard dispatching practice, this code will throw an Exception very much to the surprise of the person using your code.

Fazio