Initializeng events vs checking for null

Question

Basically this has been in my head for a while... and I'd like to read your opinion

I read the great book from Jon Skeet called: C# in Depth, Second Edition and there's a recommendation to use something like this when declaring custom events:

public event Action<string> MyEvent = delegate { };

This declaration will release us from the nullity check statement before firing the event, so instead of this:

    if (this.MyEvent != null)
    {
        this.MyEvent("OMG osh");
    }

We can simply call:

this.MyEvent("OMG osh");

And our code simply will work.

When you declare events like this, the event will be initialized with an empty delegate so we do not have the need to check for null.

This is another way to declare events, they are equivalent

private Action<string> myDelegate;
public event Action<string> MyEvent
{
    add
    {
        this.myDelegate += value;
    }
    remove
    {
        this.myDelegate -= value;
    }
}

For more info: http://csharpindepth.com/Articles/Chapter2/Events.aspx

I just had a discussion with some fellows, at work and we were discussing this topic, they were arguing that sometimes we would have the need to clear all the subscriptions for the event at once, we can simply do that assigning null to the delegate behind and if we want to keep using the same pattern we would have to re-initialize the event with an empty delegate. They were also asking what would happen in a multi-threading scenario, that's the main reason I placed this question

Question

I would like to know if there are some hidden implications (perhaps when using multi-threading) when declaring events following this pattern vs checking for null
If I use this patter I'm effectively removing several if conditions, which result removing jump statements. Wouldn't this be better in overall performance terms?

Cheers

Erm ... sorry, but ... what is your question? It's all very interesting chatty stuff - but SO is a Q&A site. — ColinE, Jul 18 '12 at 07:43
Could you format the quoted text more distinctly? (Select and use the `["]` button) — H H, Jul 18 '12 at 07:44
_I would like to read your comments about this topic_ -> not the right kind of question. — H H, Jul 18 '12 at 07:45
@Jupaol I would suggest moving your question to the start of your text, then adding the rest of your commentary underneath. — ColinE, Jul 18 '12 at 07:45
I just placed the question at the beginning and the end. It was confusing people apologize — Jupaol, Jul 18 '12 at 07:48
this has been discussed [before](http://stackoverflow.com/q/786383/201088). — Eren Ersönmez, Jul 18 '12 at 07:53

score 3 · Answer 1 · answered Jul 18 '12 at 08:04

I would be less interested in the performance than the readability and expectations of the development team. You don't really want other developers who don't know the usage pattern to think, "Ah! I see someone has forgotten to use the delegate call properly and check for null"

I had always understood that to deal with multi thread scenarios you still need to make a local copy of the underlying Delegate so you could fire to all subscribers even if the list of subscribers changed during your iteration over that list as subscribers are called.

Having said that I am still wondering about using this extension method to save the boilerplate code (with overloads for multiple arguments)

I tend to prefer Action over EventHandler events if the subscribers don't need to know who fires the event.

public static class ActionExtension
{
    public static void SafeInvoke<T>(this Action<T> action, T arg)
    {
        var temp = action;
        if (temp != null)
        {
            temp(arg);
        }
    }
}

public event Action<string> InterestingEvent;
// event invoker
InterestingEvent.SaveInvoke("Boo!");

stakx - no longer contributing · Accepted Answer · 2012-07-18T08:08:57.673

First off: Are you sure that you are not prematurely optimising? Are you actually facing a performance issue because instead of a check for null, you are calling an additional empty method when the event is triggered? If not, then I'd suggest you drop this issue.

Now, to your question: I think it's fairly safe to say that a check for null is cheaper than a superfluous method invocation, so if you want the best-possible performance, then perhaps Jon Skeet's convenient delegate { } pattern is not for you.

When it comes to multi-threading, a much more important issue would be to define on which thread / in which context (e.g. SynchronizationContext) each event handler will be invoked. You should be able to leave this decision to each consumer of your event, as some event handlers won't care, other will want to forward to the right context (e.g. by means of SynchronizationContext.Post).

If you do decide on checks for null, then note that instead of this:

if (this.MyEvent != null)
{
    this.MyEvent("OMG osh");
}

it is recommended that in multi-threading scenarios, you do this instead:

Action<string> handlers = this.MyEvent;
if (handlers != null)
{
    handlers("OMG osh");
}

That is, first copy the delegate into a local variable. This is because the delegate variable could be manipulated by another thread during the check for null. (I'm not so sure this is really necessary, but this is the suggested pattern nevertheless.)

Off-topic: I think you got a minor point slightly wrong:

[W]hen you declare an event, what [is] actually happening is that you are declaring a delegate and an event.

That is correct, but it's worth noting that to the CLI, events are simply a grouping of several methods (e.g. the add, remove, raise and possibly other accessors). Pretty much everything else is actually language-specific. For example, an event in C# is nothing other than a delegate, paired with some accessor methods, and some additional constraints on what can be done with the delegate (i.e. += and -= being the only valid operations on the delegate from outside the type where the event is declared).

Initializeng events vs checking for null

2 Answers2