4

Being accustomed to VB.NET, I'm used to "just raise events". Sure, custom events differ, but with "regular" events - I don't need to check if the delegate is Nothing before raising.

With C#, I find myself repeating this pattern:

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

I was thinking that the following pattern might prove more elegant:

  1. myHandler is initialized with an empty lambda: myHandler = (sender, e) => { };
  2. myHandler is expected never to be null, so raising would simply become: myHandler(this, new EventArgs());

Would this pattern be more or less performant than the previous one? Are there other significant considerations I should take into account?

M.A. Hanin
  • 8,044
  • 33
  • 51
  • 2
    See the answers to this very similar question: http://stackoverflow.com/questions/170907/is-there-a-downside-to-adding-an-anonymous-empty-delegate-on-event-declaration – Andrew Kennan Mar 19 '12 at 07:47

4 Answers4

5

It is something extra to happen in construction, but it won't be huge overhead. One thing to watch is that some serialization frameworks like DataContractSerializer (WCF) don't run constructors or field initializers, so it may not be non-null after all. Personally, if the majority of your events are EventHandler, I might be tempted to use an extension method:

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

then:

SomeEvent.SafeInvoke(this);

although be frank, I rarely have a problem with simply using the null-check ;p

Another downside is that if you have enough events that this is an issue, you should probably be using EventHandlerList instead - and this approach won't work with EventHandlerList.

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
2

Common practice is to use protected virtual methods OnEventName where you can check if event is null and raise it:

protected virtual void OnEventName(parameters)
{
    if (EventName != null)
        EventName(this, new EventNameEventArgs(parameters);
}

This allows you to have all event-raising code in one place (no null check duplication) and override it later if needed. So I don't see any benefit of adding one dummy event handler per event over having one null check per event.

BTW shorter way to add dummy handler is myHandler = delegate {};

Sergey Berezovskiy
  • 232,247
  • 41
  • 429
  • 459
1

Don't think there is significative difference between 1st and 2nd case provided, at least not so much to consider them. On too frequent use of delegates lack of if (myHandler != null) could, by the way, gane you some performance benefits. So if you're sure the handler is never null, get rid of that control and basically you done.

Tigran
  • 61,654
  • 8
  • 86
  • 123
1

I think that the performance difference between the two approaches isn't big enough to be relevant. If anything, I would argue that a null-check is cheaper than invoking a method through a delegate, even if it's a no-op.

When talking about elegance, it should be pointed out that the RaiseEvent keyword in VB.NET is automatically expanded by the compiler to exact same construct that you have to write yourself in C#:

If (Not MyEvent Is Nothing) Then
  MyEvent.Invoke(New EventArgs())
End If

If you want to avoid repeating that construct throughout your code, you could encapsulate it in a couple of extension methods:

public static void RaiseEvent(this EventHandler source, object sender)
{
    if (source != null)
    {
        source.Invoke(sender, new EventArgs());
    }
}

public static void RaiseEvent<T>(this EventHandler<T> source, object sender, T eventArgs)
    where T : EventArgs
{
    if (source != null)
    {
        source.Invoke(sender, eventArgs);
    }
}

That way you can simply say:

myEvent.RaiseEvent(this);
myOtherEvent.RaiseEvent(this, new SomeEventArgs());

which is semantically equivalent to the style used in VB.NET.

Enrico Campidoglio
  • 56,676
  • 12
  • 126
  • 154