Refactor Safe Implementation Of INotifyPropertyChanged

8 commentsWritten on August 5th, 2009 by
Categories: Silverlight

One thing that always annoyed the hell out of me when implementing INotifyPropertyChanged for Silverlight (or WPF) databinding was the fact that you have to provide the name of the property as a string. There are some cool AOP solutions to this, but if you can't use those for whatever reason, you could use LINQ's Expressions to avoid having to use strings.

We already have a base PresentationModel which has the following method:

        protected void NotifyPropertyChanged(string propertyName)

        {

            PropertyChanged(this, new PropertyChangedEventArgs(propertyName));

        }

We also have a generic PresentationModel class which inherits from this class:

    public abstract class PresentationModel<TPresentationModel> : PresentationModel

        where TPresentationModel : PresentationModel<TPresentationModel>

    {

        protected void NotifyPropertyChanged(Expression<Func<TPresentationModel, object>> expression)

        {

            var memberExpression = expression.Body as MemberExpression;

 

            if (memberExpression == null)

            {

                throw new InvalidOperationException("You must specify a property!");

            }

 

            NotifyPropertyChanged(memberExpression.Member.Name);

        }

    }

Notice that you pass an expression to the NotifyPropertyChanged method. It can be used like this:

    public class MyTestPresentationModel : PresentationModel<MyTestPresentationModel>

    {

        private string myString;

 

        public string MyString

        {

            get

            {

                return myString;

            }

            set

            {

                myString = value;

                NotifyPropertyChanged(t => t.MyString);

            }

        }

    }

That's pretty cool, but you should be able to test this easily, right? That's pretty easy to do as well:

    public class PresentationModelTest<TPresentationModel> where TPresentationModel : PresentationModel<TPresentationModel>

    {

        protected void AssertThatPropertyChangedIsCorrect<TArgument>(TPresentationModel model,

            Expression<Func<TPresentationModel, TArgument>> property, TArgument value)

        {

            var memberInfo = ((MemberExpression)property.Body).Member;

            var propertyName = memberInfo.Name;

 

            bool eventProperlyTriggered = false;

            model.PropertyChanged += (s, e) => eventProperlyTriggered = e.PropertyName.Equals(propertyName);

            typeof(TPresentationModel).GetProperty(propertyName).SetValue(model, value, null);

 

            Assert.IsTrue(eventProperlyTriggered);

        }

    }

 

    [TestClass]

    public class PresentationModelTests : PresentationModelTest<MyTestPresentationModel>

    {

        [TestMethod]

        public void NotifyPropertyChanged_WithExpression_TriggersCorrectPropertyChangedEvent()

        {

            var model = new MyTestPresentationModel();

            AssertThatPropertyChangedIsCorrect(model, m => m.MyString, "blah");

        }

    }

There we go... no more strings so refactoring won't break anything, and it's easy to test as well.

  • http://fgheysels.blogspot.com Frederik

    Although I fully understand the advantage of this (I also hate hardcoded strings in my code, and I’ve annoyed myself over this INotifyPropertyChanged implementation as well), I still think that this is some kind of cumbersome solution.
    I think that the code becomes less readable.

    For the moment, I do it like this:
    In my base class which implements INotifyPropertyChanged, I also have a method called ‘PropertyChanged’, which looks like a bit like this (from the top of my head):


    protected void PropertyChanged( string propertyName )
    {
    if( String.IsNullOrEmpty (propertyName) )
    {
    throw new ArgumentNullException (propertyName);
    }

    #if DEBUG

    if( !PropertyExists (propertyName) )
    {
    throw new ArgumentException ("No such property", propertyName);
    }

    #endif

    PropertyChanged (this, new PropertyChangedEventArgs(propertyName));
    }

    The PropertyExists method uses reflection in order to check whether the class has a property with the given name. (But I’m sure you’ve guessed this already ;) ).

  • http://davybrion.com Davy Brion

    passing the propertyname as a string was exactly what we were trying to get away from :)
    it’s actually less typing now because of the intellisense support.

  • Brian

    Interesting idea, I’m still trying to digest it.

    What kind of overhead does this create? Performance or dependencies?

    Right now, I’m just unit testing to verify the PropertyChanged event is thrown with the correct property name. (Very similiar to your AssertThatPropertyChangedIsCorrect)

  • http://davybrion.com Davy Brion

    there was only a performance penalty noticeable after triggering the event a couple of thousands times. obviously, if you’re doing that you’ve probably got bigger problems to worry about than the performance overhead of using an expression instead of a string ;)

  • Nima

    Thanks for your post!
    I think that I’m doing something wrong.As long as I pass a non struct property ( a string property for example) everything is fine but when I try to pass a struct property (an integer or a boolean) It won’t get a MemberExpression but some other type of expression (UnaryExpression for example)

  • Nima

    OK I get it. It seems that if an UnaryExpression is given we should use its Operand to get its MemberExpression.

    var memberExpression =(expression.Body is UnaryExpression? expression.Body.Operand :expression.Body) as MemberExpression;

  • http://kearon.blogspot.com sean kearon

    Hi Davy

    Great article and a great approach to avoiding strings. The approach I take is to use AOP and let PostSharp do the work at compile time. Design time you have code that looks something like:


    [NotifyChanges]
    public string Name { get; set }

    Alternatively, you can take a convention approach and put an attribute on the assembly (using a property attribute to exclude properties that should not publish notifications). I have a couple of posts linked below that illustrate the approach and there is more on the PostSharp website.

    Cheers

    Sean

    http://kearon.blogspot.com/2008/10/using-c-automatic-properties-with-xpo_27.html
    http://kearon.blogspot.com/2008/10/using-c-automatic-properties-with-xpo.html

  • Dale

    Looks like a reasonable solution. I achieved a similar effect by creating a generic version of the INotifyPropertyChanged interface and adding an extension method to that interface that raises the property changed event. The only problem with this approach is that implementing classes need to publically expose a method to raise the property changed event. Still, much better than magic strings!