Sunday, December 06, 2009

Yet another INotifyPropertyChanged with Expression Trees - Part 2

In my last post, I described a method whereby you can implement INotifyPropertyChanged with zero performance overhead and near-zero boilerplate code. The only boilerplate left was the delegate you had to create to invoke the event:

public Book()
{
// Boilerplate - eugh!
Action<string> notify = (propertyName) => {
var h = PropertyChanged;
if (h != null)
h(this, new PropertyChangedEventArgs(propertyName));
};

author = new ChangeNotifier<string> (() => Author, notify);
price = new ChangeNotifier<decimal> (() => Price, notify);
quantity = new ChangeNotifier<int> (() => Quantity, notify);
title = new ChangeNotifier<string> (() => Title, notify);
}

The entire point of my implementation was to avoid writing boilerplate, so this was slightly irritating. Unfortunately, there's no trivial way around the problem as the .NET framework really limits what you can do with events. The first thing you'd think of is "pass the actual object into the ChangeNotifier constructor and just raise the event that way". For example my constructors would change to:

new ChangeNotifier<string>(() => Author, this);

That's well and good, right up until you realise that it's impossible for one object to raise an event that's declared on another object.

public class A
{
public event EventHandler MyEvent;
}

public class B
{
public void AccessEvent (A a)
{
// Invalid - you can't raise an event which is declared in another class
a.MyEvent(this, EventArgs.Empty);

// Invalid - you can't copy the event either
EventHandler h = a.MyEvent;
h(this, EventArgs.Empty);
}
}
Another alternative would be to pass the event itself into the ChangeNotifier object:
new ChangeNotifier<string> (() => Author, PropertyChanged);
But this won't work because a copy of the delegate list is created. That means if anyone adds event handlers later on, they won't be invoked when the property changes. So with that stuck firmly in my mind, I never gave much thought to removing that last remaining bit of boilerplate. That's about to change!

What I really want is for my final implementation to look more like this:

public class Book : INotifyPropertyChanged
{
public event PropertyChangedEventHandler PropertyChanged;

ChangeNotifier<string> author;

public string Author
{
get { return author.Value; }
set { author.Value = value; }
}

public Book()
{
author = ChangeNotifier.Create(() => Author, ????);
}
}

That's short and sweet . The generic types should be automatically inferred, you shouldn't have to create the delegate to raise the event, it's beautiful! The only problem is to figure out what I should replace the question marks with. I need something that will allow me to get at the current list of event handlers from outside of the Book object, i.e. something along the lines of this:

Func<PropertyChangedEventHandler> getter = delegate { return PropertyChanged; };

Prettying it up a little, this is how my Book class looks:

public class Book : INotifyPropertyChanged
{
public event PropertyChangedEventHandler PropertyChanged;

ChangeNotifier<string> author;

public string Author {
get { return author.Value; }
set { author.Value = value; }
}

public Book()
{
author = ChangeNotifier.Create (() => Author, () => PropertyChanged);
}
}

Beautiful! The more astute readers might notice a problem at this stage. Fine, the ChangeNotifier object can get the event list and raise the event, but it can't fill in the 'sender' - it has no reference to the 'book' object! Have no fear, it's already taken care of! The getter delegate has a reference to the book object (Delegate.Target), so we can fill everything in perfectly! The final implementation of the ChangeNotifier class is this:
public static class ChangeNotifier
{
public static ChangeNotifier<TValue> Create<TValue>(Expression<Func<TValue>> expression, Func<PropertyChangedEventHandler> notifier)
{
return new ChangeNotifier<TValue>(expression, notifier);
}
}

public class ChangeNotifier<TValue>
{
Func<PropertyChangedEventHandler> notifier;
string propertyName;
TValue value;

public TValue Value {
get { return value; }
set {
if (!EqualityComparer<TValue>.Default.Equals(this.value, value)) {
this.value = value;
// Get the current list of registered event handlers
// then invoke them with the correct 'sender' and event args
PropertyChangedEventHandler h = notifier();
if (h != null)
h(notifier.Target, new PropertyChangedEventArgs(propertyName));
}
}
}

public ChangeNotifier(Expression<Func<TValue>> expression, Func<PropertyChangedEventHandler> notifier)
{
if (expression.NodeType != ExpressionType.Lambda)
throw new ArgumentException("Value must be a lamda expression", "expression");
if (!(expression.Body is MemberExpression))
throw new ArgumentException("The body of the expression must be a memberref", "expression");

MemberExpression m = (MemberExpression)expression.Body;
this.notifier = notifier;
this.propertyName = m.Member.Name;
}
}
I have one final trick up my sleeve. Suppose you have a field (Progress) whose value is calculated based on other values (CurrentStep, TotalSteps) and you want to get Notifications whenever any of those fields changes, well, that's easy!

public class Worker : INotifyPropertyChanged
{
public event PropertyChangedEventHandler PropertyChanged;

ChangeNotifier<int> currentStep;
ChangeNotifier<int> totalSteps;

public int CurrentStep {
get { return currentStep.Value; }
set { currentStep.Value = value; }
}
public int TotalSteps {
get { return totalSteps.Value; }
set { totalSteps.Value = value; }
}
public double Progress
{
get { return (double)CurrentStep / TotalSteps; }
}

public Worker()
{
Func<PropertyChangedEventHandler> notifier = () => PropertyChanged;

currentStep = ChangeNotifier.Create(() => CurrentStep, notifier);
totalSteps = ChangeNotifier.Create(() => TotalSteps, notifier);

// A PropertyChanged notification will be created for Progress every time
// either the CurrentStep *or* TotalSteps changes.
ChangeNotifier.CreateDependent(
() => Progress,
notifier,
() => CurrentStep,
() => TotalSteps
);
}
}

And the new helper methods are:
public static class ChangeNotifier
{
static string GetPropertyName(Expression expression)
{
while (!(expression is MemberExpression)) {
if (expression is LambdaExpression)
expression = ((LambdaExpression)expression).Body;
else if (expression is UnaryExpression)
expression = ((UnaryExpression)expression).Operand;
}

return ((MemberExpression)expression).Member.Name;
}

public static void CreateDependent<TValue>(Expression<Func<TValue>> property, Func<PropertyChangedEventHandler> notifier, params Expression<Func<object>>[] dependents)
{
// The name of the property which is dependent on the value of other properties
var name = GetPropertyName(property);
// The names of the other properties
var dependentNames = dependents.Select<Expression, string>(GetPropertyName).ToArray();

INotifyPropertyChanged sender = (INotifyPropertyChanged)notifier.Target;
sender.PropertyChanged += (o, e) => {
// If one of our dependents changes, emit a PropertyChanged notification for our property
if (dependentNames.Contains(e.PropertyName)) {
var h = notifier();
if (h != null)
h(o, new PropertyChangedEventArgs (name));
}
};
}

public static ChangeNotifier<TValue> Create<TValue>(Expression<Func<TValue>> expression, Func<PropertyChangedEventHandler> notifier)
{
return new ChangeNotifier<TValue>(expression, notifier);
}
}

The only change is that I need to use a slightly more complicated method of getting the property name as it's possible for certain types to get wrapped in a ConvertExpression.

8 comments:

Unknown said...

What an elegant way of solving this problem, congratulations!

I have been trying to wrap my head around doing something like this several times but I always end up fantasizing about new language features that would make this easier.

However I still think the ultimate thing would be if properties, backing variables and their respective change event(s) could be tied together in the language and maybe the CLR, because its such a powerful pattern! :)

- Karsten

Glenn Block said...

Nice job! I explored something similar a while ago, though a slightly different approach. This is elegant, the only thing I don't like is the requirement of the extra setup code.

It's not horrible, just not optimal. On the other hand the advantage of this approach over dynamic proxy type stuff is that it supports construction through new rather than requiring you to resolve off of a container.

On the implementation of Book, most likely though you wouldn't raise the event directly, you will have a handler to ensure that it's not null etc.

That being the case you could create a derived interface that has an OnPropertyChanged on it (something that the Fluent Silverlight guys have done to address this)

Then once you have that, you can drop the need to pass the delegate over and over (PropertyChanged). Instead you just cast the instance to the interface and call OnPropertyChanged passing in the prop name.

Glenn

Jonathan Pryor said...

Two questions for you ;-)

1. What license is this under? Hopefully MIT/X11 is sufficient.

2. Would you mind if I added it to Cadenza? http://gitorious.org/cadenza

Anonymous said...

We've been playing with something similar to this - and found that it's nice to make ChangeNotifier non-static - instead of doing:

Func notifier = () => PropertyChanged;
ChangeNotifier.Create(..., notifier);

You can then do:

var notifier = new ChangeNotifier(() => PropertyChanged);
notifier.Create(() => CurrentStep);
notifier.Create(() => TotalSteps);

With lots of properties, it's quite a bit less typing.

Alan said...

@glenn:

If you mean you don't like the boilerplate of instantiating each field with a change notifier object, there are ways around that. Whether or not it's easier, who knows. One simple API is this:

ChangeNotifier.Create (
() => this.PropertyChanged,
() => this.intProperty,
() => this.floatProperty,
() => this.myOtherField
);

You could instantiate all your fields using one method call. Or you could hide this in a base class and use reflection to perform the same task. It's all up to you. Blogposts about alternative APIs would be interesting ;)


"On the implementation of Book, most likely though you wouldn't raise the event directly, you will have a handler to ensure that it's not null etc."

This is exactly what happens ;)


"Instead you just cast the instance to the interface and call OnPropertyChanged passing in the prop name."

Which would require every subclass to implement the "OnPropertyChanged" method which is boilerplate I wouldn't like. You could 'simplify' by using an instance factory (another guy though this up and is going to blog about it). Essentially:

Factory f = new Factory (() => PropertyChanged);

this.intProperty = f.Create (() => IntProperty);
this.floatProperty = f.Create (() => FloatProperty);

If you keep the names of your fields/properties consistent, you could reduce this to:

f.Create (() => FloatProperty);

which will automatically find and populate the field (floatProperty) for you.

Alan said...

@jonp:

It's MIT/X11 and feel free to add to Cadenza.

@reed:

I got your email and am looking forward to your post explaining and detailing your ideas on how to improve this :)

www.almeria-3d.com said...

Wow, there's really much effective information above!

aliyaa said...

I think that statement of purpose management information systems are now a days becoming strong event. People are taking this into account with lot of visible response on their faces.

Hit Counter