C# INotifyPropertyChanged handler

Question

I am starting to read about MVVM and one pattern I see a lot is:

public event PropertyChangedEventHandler PropertyChanged;
//.....

PropertyChangedEventHandler handler = this.PropertyChanged;
if (handler != null)
{
    var e = new PropertyChangedEventArgs(propertyName);
    handler(this, e);
}

Why to bother to declare this handler variable? It just looks to me as complicating code unnecessarily, but I can see this even at Microsoft's own tutorials, why not to just use it as:

if (this.PropertyChanged != null)
{
    var e = new PropertyChangedEventArgs(propertyName);
    this.PropertyChanged(this, e);
}

Answer 1

Storing off the PropertyChanged event is for thread-safety. In fact, you should technically be doing this with all your events.

The assignment creates a copy of the event and its handlers (not a reference, which would be useless), which means you avoid the scenario where the event handler is set to null right after it passes the null check. This avoids a potential race condition that would throw a NullReferenceException.

In reality, the UI isn't setting that property to null very often, if at all. To be safe however, and use good practice, you should assign the handler off.

Answer 2

I just wanted to add a couple of points to BradleyDotNET's answer.

In his book CLR via C#, Jeffrey Richter points out that the pattern is still not guaranteed to be thread-safe, because the compiler could optimize away the local variable (although the current version of the compiler does not make this optimization). He recommends using Volatile.Read to be technically correct:

PropertyChangedEventHandler handler = Volatile.Read(ref PropertyChanged);
if (handler != null)
{
    var e = new PropertyChangedEventArgs(propertyName);
    handler(this, e);
}

In reality though, the other pattern is so widely used that Microsoft would very likely never make a change to the compiler that would break so many applications.

---

One other point, is that you can actually add an empty dummy delegate to your events, like below:

public event PropertyChangedEventHandler PropertyChanged = delegate { };

This means you don't have to worry about any null checking, because the event has at least one handler attached to start with. This practice is controversial, though, since it would seem to incur a performance penalty (in theory you add an extra invocation to every event call). Personally I avoid using it, as I would rather suffer more bloated code than bloated runtime performance.

---

This blog post by Eric Lippert on the topic is a good read.