Which is the better approach to invoke an event delegate?

Question

private void NotifyFreeChannelsChanged() //1.
{
    if (FreeChannelsChanged != null)
    {
        FreeChannelsChanged(this, null);
    }
}

private void NotifyFreeChannelsChanged() //2.
{
    NotifyCollectionChangedEventHandler h = FreeChannelsChanged ;
     if (h != null)
         h(this, e);
}

out of these which is better and why. or it is just a extra check. not a major difference.

Answer 1

"Better"? Well, doesn't contain a (specific) race condition, one does. MultiCastDelegate types are immutable and use value type semantics in all of the ways that matter (they are reference types however, see this and, more importantly, this), that's why you assign it first and then check. The problem is that:

// this evaluates to true...
if(SomeEvent != null)
{
    // ...but before this line executes, the last 
    // subscriber detached, and now SomeEvent is null. Oops.
    SomeEvent(this, e);
}

You should have asked "why would anyone use example #2?"

---

As an aside, this is a great place to use an implicitly typed variable (var). Those delegate type names get long...

Also interesting is that a race condition still exists, it's just more subtle. What happens if a subscriber is removed after the assignment? Well, it will still get called, but there's really nothing (I know of) that you can do about it.

Answer 2

The second one lends itself easily to a set of extension methods that we use, where the EventHandler is the target of the method.

public static void Raise(this EventHandler handler, object sender)
{
  if (handler != null)
    handler(sender, EventArgs.Empty);
}

// And then...
TheEvent.Raise(this);

As far as I know, the fact that you pass the handler to a method gives you that local copy to avoid a race condition.