0

I am kind of tired of having all this useless noise in my code:

private void RaiseSomeOtherEventIfItIsNotNull()
{
    if (this.SomeOtherEvent != null)
    {
        this.SomeOtherEvent(this, EventArgs.Empty);
    }
}

In 99.9% of the cases I don't care if someone attached to it or if it is null or not. Just raise the event! I really don't get it why the c# compiler makes me write all this noise.

So I though I could maybe declare an event like this:

public event EventHandler SomeOtherEvent = delegate { };

this would allow me to get rid of the useless null check and the useless Raise* Method. I could just always do:

this.SomeOtherEvent(this, EventArgs.Empty);

Now when I compare the standard approach with "my" approach in Lutz Röder's Reflector I see some signigicant differences. The compiler has overriden Add{} and Remove{}there is an extra static instance of the anonymous delegate:

[CompilerGenerated] 
private static EventHandler CS$<>9__CachedAnonymousMethodDelegate1;

and there is this:

.method private hidebysig static void <.ctor>b__0(object, class [mscorlib]System.EventArgs) cil managed
{
    .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor()
    .maxstack 8
    L_0000: nop 
    L_0001: ret 
}

Now my question: Do you seen any issues or disadvantages in decalring events with a default initialization like this?

bitbonk
  • 48,890
  • 37
  • 186
  • 278
  • 1
    possible duplicate of http://stackoverflow.com/questions/170907/is-there-a-downside-to-adding-an-anonymous-empty-delegate-on-event-declaration – Dyppl Feb 18 '11 at 09:53
  • Yes, you are right, lets close this one. – bitbonk Feb 18 '11 at 10:12

4 Answers4

7

You've shown an extra method, but not the extra class. The extra method is fine, IMO - that's just representing the no-op handler. Not a problem.

An extra class is somewhat surprising, as is the idea that it's changing the add/remove behaviour... the compiler would always be creating add/remove methods for the event, as that's what makes it an event.

Personally I think this is fine - but an alternative would be to write extension methods, e.g.

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

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

then just call

myEvent.Raise(this, args);

That will work for all EventHandler and EventHandler<T> events.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • To be fair, I *mentioned* the extra class. Or is there another one I didn't see. – bitbonk Feb 18 '11 at 10:05
  • @bitbonk: Yes, you mentioned the extra class but didn't say anything about what it was. That's what I was surprised about - that there supposedly *is* an extra class. What's in it? – Jon Skeet Feb 18 '11 at 10:17
  • Ok it is not a extra class: `[CompilerGenerated] private static EventHandler CS$<>9__CachedAnonymousMethodDelegate1; ` It's just an instance of the EventHandler delegate (wich is a class:) ) – bitbonk Feb 18 '11 at 15:15
  • @bitbonk: No, that's not a class at all. It's a *field*... which is just used to cache the delegate so that only one is ever created, however many instances of the class you create. – Jon Skeet Feb 18 '11 at 15:16
1

The convention used by microsoft is this :

Declare the event like this.

public event EventHandler<ChangedArgs> ItemChanged; 

Then create a calling method

protected virtual void OnItemChanged(ChangedArgs args) 
{
     var handler = ItemChanged; 

     if (handler != null) 
         handler(this, args);
}

This also is ThreadSafe and whenever to raise the event, simply call OnItemChanged

Joep Killaars
  • 1,096
  • 2
  • 6
  • 5
0

As far as I can tell, no, since these are no-op handlers (since the IL generates a nop and a ret instruction). Assigning an empty handler is pretty much the same as having none at all, i.e. you do nothing about that event.

BoltClock
  • 700,868
  • 160
  • 1,392
  • 1,356
  • 1
    Well there is one difference: There will be one useless EventHandler.Invoke() wich might be slightly more expensive than a null check. – bitbonk Feb 18 '11 at 09:56
0

I could be completely wrong (and if so, do tell me why, I'm curious too now), but maybe you need to declare the event delegate at class level?

public delegate void SomeEvent(object sender, EventArgs e); 

?

James Love
  • 1,025
  • 9
  • 16