public event EventHandler SelectedNodeChanged;

    protected virtual void OnSelectedNodeChanged(object sender, EventArgs e)

    {

        //Event will be null if there are no subscribers

        if (SelectedNodeChanged != null)

        {

            SelectedNodeChanged(this, e);

        }

    }

    public event EventHandler SelectedNodeChanged;

    protected virtual void OnSelectedNodeChanged(object sender, EventArgs e)

    {

        //Make a temporary copy of the event to avoid possibility of

        //a race condition if the last subscriber unsubscribes

        //immediately after the null check and before the event is raised.

        EventHandler handler = SelectedNodeChanged;

        //Event will be null if there are no subscribers

        if (handler != null)

        {

            handler(this, e);

        }

    }

There is a much better technique which you can use in C# 2 and higher. Whenever you declare an event field in code, initialize it to a do-nothing delegate, instead of leaving it uninitialized. For example:

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

Then whenever you use that delegate, it is not necessary to test for null and it is also not necessary to make a copy of it in multithreaded code. In both cases, it is sufficient to:

SomethingHappened(this, EventArgs.Empty);

It is only in the case where an event field might be uninitialized that you have to check for null and that you have to copy the delegate in multithreaded code. It is better to avoid allowing the event field to be uninitialized in the first place.

Jay 3/2/2008 12:49:37 AM

Jay, I like the idea, but it places the onus on developers always initializing the event. IMHO that's too risky.

We just created an EventsHelper which makes firing events a single line of code that can be re-used everywhere.

EventsHelper.Fire(someEvent, this, EventArgs.Empty);
...
public static void Fire<TEventArgs>(EventHandler<TEventArgs> delegates, object sender,
TEventArgs args) where TEventArgs : EventArgs
{
// Make a temporary copy of the event to avoid possibility of
EventHandler<TEventArgs> handler = delegates;

if (handler != null)
{
handler(sender, args);
}
}

Si 3/5/2008 3:36:41 AM

Si, that places the onus on developers always firing an event delegate in a manner radically different from the default, standard, and documented way of firing all other delegates - especially now when we are transitioning into a C# 2/3 where delegates are first-class objects which we create, pass, return, and call all over the place.

An automatically checked rule (perhaps a regular expression run by a precommit hook or an FxCop rule) can require that all event delegate fields be initialized when they are declared. Such a rule seems sufficient to eliminate the risk of attempting to invoke a null event delegate field.

Jay 3/5/2008 4:42:42 AM

Jay's comment content has been covered plenty. Here is one such reference that I think deserves proper credit:

http://devlicio.us/blogs/rob_eisenberg/archive/2008/03/20/net-event-techniques.aspx

Guillermo Salas 3/27/2008 1:11:07 AM

The technique I posted has indeed been covered on various blog articles. I have seen it around for a long time, and I no longer remember where I first found it.

The pattern is basically an application of the maxim "always initialize your variables when you declare them". Here the variable is simply an event field, having type delegate - but it should still be initialized when it is declared.

Jay 3/27/2008 1:32:12 AM

Jay, I have no problem with your maxim - so long as everyone remembers to embrace it! (my original point), and from that blog post the performance costs seem negligible, but I'm confused about why you say my approach is radically different from the default/standard/documented way?

I'm just using generics and a static helper class to avoid repeating code whenever you want to fire an event.

Have a look at:
http://msdn2.microsoft.com/en-us/library/w369ty8x.aspx

Here's our implementation, replacing the static helper with inline code:

...
[NonSerialized]
EventHandler<EventArgs> _someEvent;
...
public event EventHandler<EventArgs> SomeEvent
{
add { lock (this) { _someEvent += value; }}
remove { lock (this) { _someEvent -= value; }}
}
...
private void someMethod()
{
EventHandler<TEventArgs> handler = _someEvent;
if (handler != null) { handler(this, EventArgs.Empty); }
}

I'm also confused as to how that approach stops you from doing the other things you mention? Along with being thread-safe and a single line of code to invoke any event, tests are easy:

event EventHandler<EventArgs> simpleEvent;
event EventHandler<TestEventArgs> testEvent;
string _message;

void testEventHandler(object sender, TestEventArgs args)
{
_message = args.Message;
}


/// <summary>
/// Verify that Fire doesn't through an exception when there are no subscribers.
/// </summary>
[Test]
public void Fire_DoesntThrowNullReferenceException_NoSubscribers{
simpleEvent = null;

EventsHelper.Fire<EventArgs>(simpleEvent, this, EventArgs.Empty);
}

/// <summary>
/// Verify that Fire works when no events arguments are declared.
/// </summary>
[Test]
public void Fire_EventsInvoked_EventArgsEmpty()
{
int count = 0;
simpleEvent += delegate { count++; };
simpleEvent += delegate { count++; };
EventsHelper.Fire<EventArgs>(simpleEvent, this, EventArgs.Empty);
Assert.AreEqual(2, count);
}

/// <summary>
/// Verify that Fire works when event arguments are declared.
/// </summary>
[Test]
public void Fire_EventsInvoked_EventArgsNotEmpty()
{
_message = string.Empty;
testEvent += testEventHandler;
EventsHelper.Fire<TestEventArgs>(testEvent, this, new TestEventArgs("bogus"));
Assert.AreEqual("bogus", _message);
}

Si 3/27/2008 4:24:05 AM

Can someone explain to me how:
EventHandler handler = SelectedNodeChanged;

makes a copy, I thought that syntax (with reference types) would just leave you with handler as a reference to the same instance as SelectedNodeChanged

What am I missing?

thanks

Tom 6/12/2008 3:01:32 AM

2 Tom: event is kind of reference to delegate(s). And that statement makes your own copy of that reference. So even if the last subscriber unsubscribes right after the null check you still have your own reference to that subcriber.

kostya.ly 8/24/2008 8:52:22 PM

All the solutions provided above are correct, but you should consider the situation where one of the subscribers throws an exception. In that case, all delegate methods following the erroneous subscriber will not be fired. To work around this problem, you should use the following piece of code:

// The ControlChanged event will be null if there are no subscribers.
if (SelectionChanged != null) {
&nbsp;&nbsp;Delegate[] list = SelectionChanged.GetInvocationList();
&nbsp;&nbsp;foreach (Delegate del in list) {
&nbsp;&nbsp;&nbsp;&nbsp;try {
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;EventHandler handler = (EventHandler) del;
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;handler(this, EventArgs.Empty);
&nbsp;&nbsp;&nbsp;&nbsp;}
&nbsp;&nbsp;&nbsp;&nbsp;catch { }
&nbsp;&nbsp;}
}

Bert Loedeman 10/10/2008 4:36:44 PM

Excuse me for the &nbsp; 's. I thought I would make things prettier to read, but I didn't.

Bert Loedeman 10/10/2008 4:38:25 PM

I've subsequently found out that the real answer to my original question is that delegates are immutable so I was right when I said with a = b both a and b end up pointing to the same object, the clever bit is that the delegate add and remove methods actually return a new delegate type leaving the existing one untouched.

Tom 10/11/2008 3:42:53 PM

This is so cool..thanks so much for the info. Love it!

ADDIO CELIBATO ROMA 6/30/2010 12:15:00 PM