Is this a good idea for an extension method?

Question

Often I see code like this scattered and duplicated around source code:

var handler = MyEvent;

if (handler != null)
{
    handler.Invoke(null, e);
}

Is there any reason not to just encapsulate it within an extension method like this?

public static void SafeInvoke<T>(this EventHandler<T> theEvent, object sender, T e) where T : EventArgs
{
    var handler = theEvent;

    if (handler != null)
    {
        handler.Invoke(sender, e);
    }
}

So that calls can just be made like so:

MyEvent.SafeInvoke(this, new MyEventArgs(myData));

Hmm.. okay. I have just never seen this extension method before and was wondering why people always duplicate the code rather than doing this. — Ryan Peschel, Sep 29 '11 at 03:48
probably because it is trivial and will never change, so the only "bad" part about the duplication is extra code. You can also just set the event equal to an empty delegate at declaration and there is no longer a need for the null check, i.e., `public event SomeEventHandler Foo = delegate { };`. — Ed S., Sep 29 '11 at 03:50
@EdS.: Check the comments for [this](http://stackoverflow.com/questions/9033/hidden-features-of-c/9282#9282) post for why that is a bad idea. — Ryan Peschel, Sep 29 '11 at 03:55
@Ryan: I don't buy the "If you don't fire the event there is added overhead" argument because, if I don't want to fire the event, I will just delete it! However, it is an interesting point and I didn't know that was the case (and most of us don't write public API's, worrying about breaking the interface). It also shows how often I serialize my classes as I have never run into that problem. Again though, knowledge is good, and I didn't know that the JIT'er had a special case for the assignment + null check for thread safety, and all of those are fair points, so thanks! — Ed S., Sep 29 '11 at 06:29

score 3 · Accepted Answer · answered Sep 29 '11 at 05:40

This is an interesting one. Jeffrey Richter talsk a lot about this in CLR via C#.

With a null check like the following:-

var handler = MyEvent;

if (handler != null)
{
    handler.Invoke(null, e);
}

The JIT compiler could potentially optimise away the handler variable entirely. However, the CLR team are aware that many developers use this pattern for raising events and have built this awareness into the JIT compiler. It is likely that this will remain in place in all future versions of the CLR because changing the behaviour might break too many existing applications.

There is no reason that you can't write an extension method to do this work for you (this is exactly what I have done). Note that the method itself may be JIT inlined (but the temporary variable still won't be optimised away).

If you envisage your application being used against another runtime, e.g. Mono (which I suspect mirrors the CLR's behaviour in this case anyway) or some other exotic runtime which may appear, then you can guard your code against the possibility of JIT inlining by adding [MethodImplAttribute(MethodImplOptions.NoInlining)] to your extension method.

If you decide not to use an extension method, another way to protect against JIT optimisation (which Jeffrey Richter recommends) is to assign the value to the temporary variable in a way which cannot be optimised away by the JIT compiler, e.g.

var handler = Interlocked.CompareExchange(ref MyEvent, null, null);

score 0 · Answer 2 · answered Sep 29 '11 at 04:29

0

Maybe because of Performance considerations.

answered Sep 29 '11 at 04:29

Panos

991
4
5

At the end of the article the extension method performs at either the same or even a faster execution time than the normal method (maybe because of caching). The empty delegate is the only one with performance issues and I'm not doing that. – Ryan Peschel Sep 29 '11 at 04:35
I didn't say there was a problem, only considerations :)! And yes, the only black sheep is the empty delegate. – Panos Sep 29 '11 at 04:46

score 0 · Answer 3 · answered Sep 29 '11 at 04:35

You are free to create such an extension method if the pattern bothers you.

The only downsides I can see is the added dependency on the namespace that your extension method class resides in, and the fact that other developers will need to waste 3 to 5 seconds of their lives working out what SafeInvoke does.

Is this a good idea for an extension method?

3 Answers3