06/12/2015

An everyday task

If you've written some MVVM Wpf Application, chances are you've probably implemented the interface INotifyPropertyChanged a couple of times. In this post, I am going to talk about different options for implementing this simple interface.

The interface looks like this:

INotifyPropertyChanged
interface INotifyPropertyChanged
{
  event PropertyChangedEventHandler PropertyChanged;
}

The Requirement

Now consider implementing a class that has a simple property that raises the PropertyChanged event when it changes. Let's specify the capabilities of the class using the following interface:

interface IChangeNotifier : INotifyPropertyChanged
{
  int Value { get; set; }
}

A straightforward implementation

Let's take a stab at it with a straightforward implementation:

class ChangeNotifier : IChangeNotifier
{
  private int _Value;
  public int Value
  {
    get { return _Value; }
    set
    {
      _Value = value;
      OnPropertyChanged(new PropertyChangedEventArgs("Value"));
    }
  }
  public event PropertyChangedEventHandler PropertyChanged;
  protected virtual void OnPropertyChanged(PropertyChangedEventArgs e)
  {
    if (PropertyChanged != null)
      PropertyChanged(this, e);
  }
}
While this implementation satisfies the requirements of raising the event when the value changes, it brings with it a list of rather suboptimal consequences.

  • Setting the property fires the event everytime, even if the supplied value has not changed.
    1. Besides being a possible performance problem, as the event is invoked multiple times,
    2. when used with event handlers that update properties themselves a deadlock can be triggered. eg. consider two input fields that display a number in two different formats. If the first number is updated by the user, the second number is adjusted by the event handler and vice-versa. If the events keep triggering themselves the application will deadlock.
  • The code is not exception safe if the unsubscriber and the firing of the event happen on different threads.
    Consider the race condition that could occur when the thread that fires the event checks 'if (PropertyChanged != null)' and then a different thread unsubscribes in just that moment and the first thread tries to raise the event and triggers a NullReferenceException instead.
  • The code is not refactoring safe for the author.
    If (better: when) the author of the class renames the property he has to remember to rename the hardcoded string as well. Consider refactoring the interface - a rename of the interface property in Visual Studio will rename the property names of all implementing classes, but not the string used in with firing the PropertyChangedEventHandler.
  • The code is not typo safe for the consumer.
    Consider the consumer of the event, who is probably trying to discern in his eventhandler which property raised the event. If he makes a typo, the compiler will be unable to catch the error.

An improved version

First, we'll modify the property setter to fire only when the value of the property changes.

private int _Value;
public int Value
{
  get { return _Value; }
  set
  {
    if (_Value != value)
    {
      _Value = value;
      OnPropertyChanged(new PropertyChangedEventArgs("Value"));
    }
  }
}
That solves the first of our concerns - the event handler is now only fired if a change of the value really happens.

Now let's redo the event implementation to fix the race condition:

public event PropertyChangedEventHandler PropertyChanged;
protected virtual void OnPropertyChanged(PropertyChangedEventArgs e)
{
  var propertyChanged = this.PropertyChanged;
  if (propertyChanged != null)
    propertyChanged(this, e);
}

By storing a local copy of the event handler, we make sure to test and execute the same instance, hence solving the possible NullReferenceException. We're able to cross our second concern off the list.

Further Options

We still have the problem of refactoring for the author and consumer of the class. Let's try to tacke those.

Using a const field

Then there is the option of using a const field to hold the string:

public const string ValuePropertyName = "Value";
private int _Value;
public int Value
{
  get { return _Value; }
  set
  {
    if (_Value != value)
    {
      _Value = value;
      OnPropertyChanged(new PropertyChangedEventArgs(ValuePropertyName));
    }
  }
}
This does not solve your refactoring problem, but the author of the class and the event consumer at least use the same field and therefore the consumer can't make a typo.

The drawback here being, that

  1. the consumer of the event must write his code against the actual implementation of the class to access the ValuePropertyName field and
  2. each implementor of the interface gets to pick a (possibly wrong) name.
So ideally the constant string would be part of the interface, which is not possible. This brings as to our next attempt:

Using an extension Method

Putting it into an extension class for the interface works kind of, the trouble being that extension properties are not supported and neither const nor static, so your only option is a instance method.

Consider the extension method defined in:

public static class ChangeNotifierExtensions
{
  public static string GetValuePropertyName(this IChangeNotifier changeNotifier)
  {
    return "Value";
  }
}

Now you can imagine every implementor of the IChangeNotifier interface as well as the event consumer accessing the same string - if they follow our convention.

Using Linq's MemberExpression

It's possible to substitute the hard coded string using a strongly typed property reference using the Linq's MemberExpression. Take a look at the following code:

private int _Value;
public int Value
{
  get { return _Value; }
  set
  {
    if (_Value != value)
    {
      _Value = value;
      OnPropertyChanged(() => this.Value);
    }
  }
}
protected virtual void OnPropertyChanged(Expression> property)
{
  string propertyName = (property.Body as MemberExpression).Member.Name;
  OnPropertyChanged(new PropertyChangedEventArgs(propertyName));
}

public event PropertyChangedEventHandler PropertyChanged;
protected virtual void OnPropertyChanged(PropertyChangedEventArgs e)
{
  var propertyChanged = this.PropertyChanged;
  if (propertyChanged != null)
    propertyChanged(this, e);
}

This does what we want - we got rid of the string "Value" and we're refactoring save - if the name of the property get's refactored the string used to create the PropertyChangedEventArgs instance is updated as well. The consumer of the event can even use a similiar method to test for the properties name.

So we're done, right?

Well, of course there is a catch here (despite depending on a newer framework than the former option) and it's performance related. Using the approach with the MemberExpression class is significantly slower than the hard coded string.

Does it matter? The answer is of course: it depends. If you're only firing a couple of events, the delay is negliable. But if your firing hundred thousand it adds up. And there is one-time delay on the first execution of the path. So don't use it, if you're implementing your game loop and consider warming it up if you need fast first-time performance.

Additional performance considerations

There is another possible perfomance cost payed in the usual implementation and it's memory related. Everytime we raise the event a new PropertyChangedEventArgs instance is allocated. And it's unnecessary, as everytime we create it with the same string for the same property.

Does this really matter? Again it depends - it did for me when I was implementing a Xamarin Forms Android application and the opengl view stuttered occasionally, which was caused by the garbage collector pauses from creating a ton of PropertyChangedEvents.

Consider the following property implementation:

private static PropertyChangedEventArgs ValuePropertyChangedEventArgs = new PropertyChangedEventArgs("Value");
private int _Value;
public int Value
{
  get { return _Value; }
  set
  {
    if (_Value != value)
    {
      _Value = value;
      OnPropertyChanged(ValuePropertyChangedEventArgs);
    }
  }
}

Which get's rid of the unecessary allocations by reusing the same static instance of the PropertyChangedEventArgs class.

So are we doomed with flawed implementations of a pretty simple interface? No, to the rescue comes C# 6!

New features in C# 6

C# 6 comes with new features, of which one is the null-conditional operator, which is a succinct way to solve our NullReference race condition. Instead of:

protected virtual void OnPropertyChanged(PropertyChangedEventArgs e)
{
  var propertyChanged = this.PropertyChanged;
  if (propertyChanged != null)
    propertyChanged(this, e);
}
We can write:
protected virtual void OnPropertyChanged(PropertyChangedEventArgs e)
{
  PropertyChanged?.Invoke(e);
}
instead.

While the null conditional operator is just syntactic sugar, the nameof operator will solve our problem of getting a string of the property name and the property implementation will probably look like:

private int _Value;
public int Value
{
  get { return _Value; }
  set
  {
    if (_Value != value)
    {
      _Value = value;
      OnPropertyChanged(nameof(Value));
    }
  }
}

Conclusion

  1. Implementing a simple interface brings with it a couple of tradeoffs. Even if those tradeoffs are mostly minor, there are edge cases one should be aware of that warrent choosing one implementation over the other.
  2. C# 6 will make everything better. :)
Please leave a comment if something is unclear or you spot an error. Thank you for reading!

Take care,
Martin

References

Last updated 06/14/2015 13:55:51
blog comments powered by Disqus
Questions?
Ask Martin