Currently "Avoid checking for null event handlers" is at the top of the answers to the post titled Hidden Features of C# and it contains severely misleading information.
While I understand that Stack Overflow is a "democracy" and the answer rose to the top by virtue of public voting, I feel that a lot of people that up-voted the answer either did not have a complete understanding of C#/.NET or did not take the time to fully understand the consequences of the practice described in the post.
In short the post is advocating the use of the following construct to avoid having to check for null when invoking the event.
public event EventHandler SomeEvent = delegate {};
// Later..
void DoSomething()
{
// Invoke SomeEvent without having to check for null reference
SomeEvent(this, EventArgs.Empty);
}
At a first glance this may seem like a clever shortcut but it can be the cause for some serious headaches in a large application, especially if concurrency is involved.
Before calling the delegate of an event you have to check for a null reference. Just because you have initialized the event with an empty delegate does not mean that a user of your class won't set it to null at some point and break your code.
Something like this is typical:
void DoSomething()
{
if(SomeEvent != null)
SomeEvent(this, EventArgs.Empty);
}
But even in the above example the possibility exists that while DoSomething() may be run by a thread, another could remove the event handlers, and a race condition could ensue.
Assume this scenario:
Thread A. Thread B. ------------------------------------------------------------------------- 0: if(SomeEvent != null) 1: { // remove all handlers of SomeEvent 2: SomeEvent(this, EventArgs.Empty); 3: }
Thread B removes the event handlers of the SomeEvent event after the code that raises the event has checked the delegate for a null reference, but before it called the delegate. When the SomeEvent(this, EventArgs.Empty); call is made, SomeEvent is null and an exception is raised.
To avoid that situation, a better pattern to raise events is this:
void DoSomething()
{
EventHandler handler = SomeEvent;
if(handler != null)
{
handler(this, EventArgs.Empty);
}
}
For an extensive discussion of the topic of EventHandlers in .NET I suggest reading "Framework Design Guidelines" by Krzysztof Cwalina and Brad Abrams, Chapter 5, Section 4 - Event Design. Especially the discussions of the topic by Eric Gunnerson and Joe Duffy.
As suggested by Eric, in one of the answers below, I should point out that a better synchronization solution could be devised that would take care of the problem. My goal with this post was to raise awareness and not to provide a one-and-only-true solution to the problem. As suggested by Eric Lippert and by Eric Gunnerson in the above mentioned book, the particular solution to the problem is up to the programmer but what is important is that the issue not be disregarded.
Hopefully a moderator will annotate the answer in question so that unsuspecting readers won't be mislead by a bad pattern.