Initializeng events vs checking for null [closed]

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

1. 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

2. 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

Answer 1

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!");