Refactor Safe Implementation Of INotifyPropertyChanged
Posted by Davy Brion on August 5th, 2009
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.
August 7th, 2009 at 1:27 am
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
).
August 7th, 2009 at 6:46 am
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.
August 18th, 2009 at 8:38 pm
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)
August 18th, 2009 at 8:43 pm
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
August 29th, 2009 at 7:34 am
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)
August 29th, 2009 at 7:47 am
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;
September 3rd, 2009 at 5:12 pm
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