Is calling an extension method on a "null" reference (i.e. event with no subscribers) evil?

Evil or not evil?

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

// Usage:
MyButtonClicked.Raise(this, EventArgs.Empty);

// This works too! Evil?
EventHandler handler = null;
handler.Raise(this, EVentArgs.Empty);

Note that due to the nature of extension methods, MyButtonClicked.Raise will not throw a NullReferenceException if MyButtonClicked is null. (E.g. there are no listeners to MyButtonClicked event).

Evil or not?

Not evil. I wish events worked this way by default. Can someone explain why an event with no subscribers is null?

You can always declare your events like this (not that i recommend it):

public event EventHandler<EventArgs> OnClicked = delegate { };

That way they have something assigned to them when you call them, so they don't throw a null pointer exception.

You can probably get rid of the delegate keyword in C# 3.0...

Don't forget to use [MethodImpl(MethodImplOptions.NoInlining)], else its possible that it isn't thread safe.

(Read that somewhere long ago, remembered it, googled and found http://blog.quantumbitdesigns.com/tag/events/ )

Coming from a java background this has always seemed odd to me. I think that no one listening to an event is perfectly valid. Especially when listeners are added and removed dynamically.

To me this seems one of C#'s gottchas that causes bugs when people don't know / forget to check for null every time.

Hiding this implementation detail seems a good plan as it's not helping readability to check for nulls every single time. I'm sure the MSFTs will say there's a performance gain in not constucting the event if no one is listening, but imho it is vastly outweighed by the pointless null pointer exceptions / reduction in readability in most business code.

I'd also add these two methods to the class:

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

    public static void Raise<TA>(this EventHandler<TA> handler, object sender, TA args)
        where TA : EventArgs
    {
        if (handler != null)
        {
            handler(sender, args);
        }
    }

Why would it be evil?

Its purpose is clear: It raises the MyButtonClicked event.

It does add a function call overhead, but in .NET it will either be optimized away or pretty fast anyway.

It is slightly trivial, but it fixes my biggest complaint with C#.

On the whole, I think it's a fantastic idea, and will probably steal it.

I wouldn't say it's evil, but I'm interested in how your extension method fits in with the

protected virtual OnSomeEvent(EventArgs e){ }

pattern and how it handles extensibility via inheritance. Does it presume all subclasses will handle the event instead of override a method?

Although I wouldn't describ it as evil, it still has a negative implication, as it adds unnecessary overhead:

When calling

myEvent.Raise(this, new EventArgs());

the object EventArgs is initialized in all situations, even if no-one subscribed to myEvent.

When using

if (myEvent!= null) {
   myEvent(this, new EventArgs());
}

EventArgs is only initialized if someone subscribed to myEvent.

Throwing an exception when there are no handlers is not really preferable by the most. If it does not have an handlers it is better to be empty rather than null.