Is this a better way to fire/invoke events without a null check in C#?

Question

Most code I have seen uses the following way to declare and invoke event firing:

public class MyExample
{
    public event Action MyEvent; // could be an event EventHandler<EventArgs>, too

    private void OnMyEvent()
    {
        var handler = this.MyEvent; // copy before access (to aviod race cond.)
        if (handler != null)
        {
            handler();
        }
    }

    public void DoSomeThingsAndFireEvent() 
    {
        // ... doing some things here
        OnMyEvent();
    }
 }

Even ReSharper generates an invoking method the way mentioned above.

Why not just do it this way:

public class MyExample
{
    public event Action MyEvent = delegate {}; // init here, so it's never null

    public void DoSomeThingsAndFireEvent() 
    {
        // ... doing some things here
        OnMyEvent(); // save to call directly because this can't be null
    }
 }

Can anyone explain a reason why not to do this? (pro vs. cons)

Answer 1

The pros and cons are:

• null checks are extremely cheap; we're talking billionths of a second. Allocating a delegate and then garbage collecting it unsuccessfully for the rest of the lifetime of the object could take maybe upwards of a few millionths of a second. Plus, you're consuming dozens of bytes more memory. Plus, every time the event fires, you get an unnecessary call to a method that does nothing, consuming even more microseconds. If you're the sort of person who cares about millionths of a second and dozens of bytes then that might be a meaningful difference; for the vast majority of cases it will not.

• You have to remember to always create the empty delegate. Is that really any easier than remembering to check for null?

• Neither pattern actually makes the event threadsafe. It is still entirely possible with both patterns for an event handler to be fired on one thread while being removed on another thread, and that means that they race. If your handler removal code destroys state that the handler needs, then it is possible that one thread is destroying that state while another thread is running the handler. Do not think that merely checking for null or assigning an empty handler magically eliminates race conditions. It only eliminates the race condition that results in dereferencing null.

Answer 2

It is a style thing for sure; however, I think most developers go for safety over style when it comes to null checks. There is nothing in this world that can guarantee that a bug will not creep into the system and that null check will continue to be unnecessary.

Answer 3

It can still be null. Consider:

    var x = new MyExample();
    x.MyEvent += SomeHandler;

    // ... later, when the above code is disposed of

    x.MyEvent -= SomeHandler;

EDIT: actually, I take it back. Having tested this, if you've used an anonymous delegate to set the handler, it doesn't look like you can clear it, so you can save the null check.

I'm not sure whether this is reliable behaviour though, or just an artifact of the language implementation...

Answer 4

The best tradeoff I've seen for simplifying event firing is the addition of an extension method. See Raising C# events with an extension method - is it bad?.