TimL on C#.NET

Raising events is a pretty fundamental thing on the .NET world. There has been quite a bit of discussion in various blogs, forums and publications about the proper way to do it. In particular, there has been a great deal of discussion about whether you should or should not copy the delegate before invoking the delegate. This post contains my 2¢ on the subject.

Consider a simple class with a simple event MyEvent. To raise the event from within the class in which it is defined, code needs to simply call the event directly as shown in SomeMethod().

class MyBaseClass
{
    public event EventHandler MyEvent;

    ...

    private void SomeMethod()
    {
        ...
        MyEvent( this, new EventArgs() );
        ...
    }
}

 

As discussed in my previous post entitled Events vs Delegates, the event keyword is basically a security measure that prevents code outside of the class from raising the event. That means that classes derived from the class that defines the event cannot raise the event; in fact, derived class cannot even do anything more than call the += and -= operators on that event.

Rather than calling the event directly as in SomeMethod() above, the defactor-standard is to define an OnEvent method to raise the event. Addditionally, such OnEvent methods are typically made virtual so that derived class can "trap" the event by simply overriding the method without actually having to attach an event handler. Then, the override method in the derived class simply calls the virtual method in the base class to actually raise the event to other code that has attached event handlers.

class MyBaseClass
{
    public event EventHandler MyEvent;

    ....

    private void SomeMethod()
    {
        ...
        OnMyEvent( new EventArgs() );
        ...
    }

    protected virtual void OnMyEvent( EventArgs args )
    {
        MyEvent( this, args );
    }
}

class MyDerivedClass : MyBaseClass
{
    ....

    protected override void OnMyEvent( EventArgs args )
    {
        // Process the event
        ....

        // Call the base class to raise the event
        base.OnMyEvent( args );
    }
}

 

In addition to allowing the derived class to trap the event without attaching an event handler, there are other bennefits of using an OnEvent method to raise the event. In particular, there are some additonal things that need to be done before actually raising the event. And, if the event was called directly from SomeMethod() instead of going through an OnEventmethod, then that logic would have to be repeated everywhere the event is raised.

For starters, there is no guarantee that there are actually any event handlers attached to the event. And, if the event is called without any attached handlers, an exception will occur. Therefore, we need to make sure the event is not null before caling it.

protected virtual void OnMyEvent( EventArgs args )
{
    // Exit if there are no attached handlers
    if (this.MyEvent == null)
    {
        return;
    }

    // Raise the event
    MyEvent( this, args );
}

 

But, there is a hidden "cotcha" in the code above. That is, if the program is threaded (as most real programs are), then it is quite possible that the MyEvent event may have had one or more attached handlers when the condition was checked, but may no longer have any attached handlers by the time MyEvent( args ) is executed.

Because of this, many people recommend copying the event into another delegate, and then checking and calling the copy in that delegate. This works because delegates are value type variables. Therefore, even if handlers have been removed from the event itself by the time the delegae is called, the delegate will still get called without excetion.

protected virtual void OnMyEvent( EventArgs args )
{
    // Make a copy of all of the handlers attached to the event
    EventHandler myEvent = this.MyEvent;

    // Exit if there are no attached handlers in the copy
    if (myEvent == null)
    {
        return;
    }

    // Raise the event
    myEvent( this, args );
}

 

At first, I was against doing the copy but I have since reformed my ways. I have always understood why people recommended doing the copy and how it worked. But, the reason I was against the copy was because I thought it was just as dangerous to call an event handler that had already been removed. For example, when using the copy, a class may have unregistered the event handler after the copy occurred but before the delegate was called, and that event handler could end up causing some kind of exception because the class no longer expected the handler to be called (variables used in the handler may have been reset to null, etc.).

But, I realized that my logic was flawed. This is because there is no way to protect against it (short of overriding the event add and remove to use synchronization). Once the delegate is called, it essentially makes it's own copy and works on that copy, anyway. Therefore, even if an event handler was removed, it can always still be called as long as execution of the delegate started before the handler was removed.

Exception handling around a delegate is also a much debated topic. Most people recommend wrapping the call to the delegate with a try...catch, and doing Debug.Assert, logging, etc. if any exceptions were thrown by the handler methods. I also recommend this.

protected virtual void OnMyEvent( EventArgs args )
{
    // Make a copy of all of the handlers attached to the event
    EventHandler myEvent = this.MyEvent;

    // Exit if there are no attached handlers in the copy
    if (myEvent == null)
    {
        return;
    }

    // Raise the event
    try
    {
        myEvent( this, args );
    }
    catch(Exception ex)
    {
        Debug.Fail(ex.Message);
        throw; // Some people do the throw, others do not
    }

}

 

Although most people recommend doing the try...catch, there is debate about whether the exception should be rethrown or not. The basic argument is this: If you do not throw the exception, then code raising the event has no idea that something bad happened. But, since code raising the event has no idea about what handler methods may be attached to the event, it is not likely that any exceptions thrown by those event handler methods could be of use to the code that raised the event.

My recommend solution is kind of a hybrid of both. Basically, I prefer to create new exception class to be thrown if an exception occurs while caling a handler method, and setting the handler method's exception as the inner exception. (I'll leave the somewhat trivial implementation of EventHandlerException to your imagination.

protected virtual void OnMyEvent( EventArgs args )
{
    // Make a copy of all of the handlers attached to the event
    EventHandler myEvent = this.MyEvent;

    // Exit if there are no attached handlers in the copy
    if (myEvent == null)
    {
        return;
    }

    // Raise the event
    try
    {
        myEvent( this, args );
    }
    catch(Exception ex)
    {
        Debug.Fail(ex.Message);
        throw new EventHandlerException("MyEvent", ex);
    }
}

 

To me, this makes the most sense because code calling OnMyEvent() can now reasonably expect to catch an EventHandlerException if it wants to, and can even look at the inner exception if it does have reason to suspect certain types of exceptions.

Of course, no matter what you do, the act of raising the exception itself can be abstracted into a separate class so that it behaves consistently for all events.

class MyClass
{
    public event EventHandler<EventArgs> MyEvent;

    protected virtual void OnMyEvent(EventArgs args)
    {
        EventHelper<EventArgs>.RaiseEvent(
            EventHadlerExceptionType.ThrowEventHandlerException,
            this,
            this.MyEvent,
            args);
    }
}

public enum EventHadlerExceptionType
{
    EatException,
    ThrowEventHandlerException,
    ThrowOriginalException
}

public static class EventHelper<TEventArgs> where TEventArgs : EventArgs
{
    public static void RaiseEvent(
        EventHadlerExceptionType exHandling,
        object sender,
        EventHandler<TEventArgs> theEvent,
        TEventArgs args)
    {
        // No need to copy handlers because delegates are
        // value types.  The delegate has, therefore, already
        // been copied when it was passed in as a parameter.

        // Exit if there are no attached handlers in the copy
        if (theEvent == null)
        {
            return;
        }

        // Raise the event
        try
        {
            theEvent(sender, args);
        }
        catch (Exception ex)
        {
            Debug.Fail(ex.Message);
            switch (exHandling)
            {
                case EventHadlerExceptionType.EatException:
                    break;

                case EventHadlerExceptionType.ThrowEventHandlerException:
                    throw new EventHandlerException(ex);

                case EventHadlerExceptionType.ThrowOriginalException:
                default:
                    throw;
            }
        }
    }
}