4

I am looking at NotifyPropertyChanged() from INotifyPropertyChanged and noticed that in the examples from Microsoft such as here:

http://msdn.microsoft.com/en-us/library/system.componentmodel.inotifypropertychanged.aspx

There is no capturing of the delegate reference first (as it says to do here for example: Use of null check in event handler)

I had a look in the auto-generated Reference.cs for my ServiceReferences and this check is done.

So my question is should I be doing this (in whatever form such as extension methods etc)? Are there any possible issues if I don't?

Community
  • 1
  • 1
Firedragon
  • 3,685
  • 3
  • 35
  • 75

1 Answers1

7

You're right, the check should be done and their example is wrong.

Below is the standard code.

private void NotifyPropertyChanged(String propertyName)
{
    var handler = PropertyChanged;
    if (handler != null)
    {
        handler (this, new PropertyChangedEventArgs(propertyName));
    }
}

Edit: A further explanation about why this is needed (and why it works)

In the MS example they do a null check directly on PropertyChanged and then invoke it. So it would be possible for PropertyChanged to become null between the null check and the invocation. By assigning the delegate to a local variable, we can ensure that we retain a reference to the delegate and it can't change between the null check and invocation.

Ray
  • 45,695
  • 27
  • 126
  • 169
  • Okay, seems the Microsoft examples I have seen are not the best then. Thanks for the response – Firedragon Oct 27 '11 at 09:01
  • they do exactly that in the MS example... am i blind? – fixagon Oct 27 '11 at 09:41
  • @fantasticfix PropertyChanged has to be copied to a local variable first – Simon Oct 27 '11 at 09:45
  • @fantasticfix Yes :) They don't assigned the delegate to a variable. So it could have changed between the null check and invoking it. – Ray Oct 27 '11 at 09:45
  • ok totally agree... just wanted to write that this wont be thread save even you avoid the nullreference. take it back :) – fixagon Oct 27 '11 at 09:48