Something is starting to bother me recently about data binding and the INotifyPropertyChanged interface. It's just that EVERY class you write ends up having to implement it (I am generally talking about writing WPF/Silverlight applications here). And that's just not sitting well with me!
It doesn't sit well because I believe it violates the principles of separations of concern. The major annoyance for me lies with calculated properties (those which are derived from another). Lets start with a simple class, Order, which contains two properties, ItemPrice and Quantity (other fields and methods omitted for sake of brevity):
public class Order : INotifyPropertyChanged
{
public decimal ItemPrice
{
get { return this.itemPrice; }
set
{
this.itemPrice = value;
this.RaisePropertyChanged("ItemPrice");
}
}
public int Quantity
{
get { return this.quantity; }
set
{
this.quantity= value;
this.RaisePropertyChanged("Quantity");
}
}
}
We need to add a property TotalPrice to this entity which will expose the total price of the order. What do we do here? This is a calculated value so should clearly be read only, and I usually implement the calculation within the property itself. What I have found myself doing many times in the past (and I have seen done in a lot of examples) is:
public class Order : INotifyPropertyChanged
{
public decimal ItemPrice
{
get { return this.itemPrice; }
set
{
this.itemPrice = value;
this.RaisePropertyChanged("ItemPrice");
this.RaisePropertyChanged("TotalPrice");
}
}
public int Quantity
{
get { return this.quantity; }
set
{
this.quantity= value;
this.RaisePropertyChanged("Quantity");
this.RaisePropertyChanged("TotalPrice");
}
}
public decimal TotalPrice
{
get { return this.ItemPrice * this.Quantity; }
}
}
This is nasty. We have had to modify our other two properties just because we have added this new one. It works, but as you add more and more dependent properties your code gets messy. Those properties shouldn't need to know that the TotalPrice property exists!
That pattern breaks down completely when you start using inheritance in your domain objects. Imagine if I now require a new type of domain object, SalesOrder, where I want to express the fact that there is a commission (expressed as a fraction) which needs to be paid to some salesperson on this order:
public class SalesOrder : Order
{
public decimal SalesCommision
{
get { return this.salesCommision}
set
{
this.salesCommisionPercentage = value;
this.RaisePropertyChanged("SalesCommision");
this.RaisePropertyChanged("TotalCommission");
}
}
public decimal TotalCommission
{
get { return this.TotalPrice * this.SalesCommission; }
}
}
Uh-oh! It's come a bit unstuck here. If the price changes in the base class, my UI is not going to redisplay the total commission! This could be great for my sales person but not so great for my accountant!
So what do we do here to get around this? We clearly can't modify our base class, so how about we listen to our own property notifications, like thus:
public class SalesOrder : Order
{
// Properties defined as previously //
protected override void RaisePropertyChanged(string propertyName)
{
base.RaisePropertyChanged(propertyName);
if (propertyName == "TotalPrice")
{
this.RaisePropertyChanged("TotalCommission");
}
}
}
Here we override the method which raises the event, check if the relevant dependent property is being raised, and if so we raise a changed event on our commission property. This works, but YUCK! Just imagine having to unit test all this! :-(
Note that we end up with the same problem if we use composition. Imagine that instead of storing the sales commission on our entity, it was actually stored on a related SalesPerson entity and we need to calculate it from there. Now we need to know that if the sales person's commission changes, the order's total commission has changed:
public class SalesOrder : Order
{
public SalesPerson SalesPerson
{
get { return this.salesPerson; }
set
{
if (this.salesPerson != null)
{
this.salesPerson.PropertyChanged -= HandleSalesPersonPropertyChanged;
}
this.salesPerson = value;
this.RaisePropertyChanged("SalesPerson");
this.RaisePropertyChanged("TotalCommission");
this.salesPerson.PropertyChanged += HandleSalesPersonPropertyChanged;
}
}
public decimal TotalCommission
{
get { return this.TotalPrice * this.SalesPerson.Commission; }
}
protected virtual void HandleSalesPersonPropertyChanged(object sender, PropertyChangedEventArgs e)
{
if (e.PropertyName == "Commission")
{
this.RaisePropertyChanged("TotalCommission");
}
}
}
We subscribe to the PropertyChanged event on our sales person, and when its commision is changed, we act accordingly.
I think this is all a mess, and it just gets worse and worse as you build up your domain model and add more classes and more relations. It's easy to forget one of these dependencies. If you are building a fairly complex UI on top of this (e.g. one which allows you to drill down and navigate through the relationships and make changes to objects), I'm so sure you will introduce bugs due to not raising the event, I know it's happened to me!
On my next (real) WPF project this is definitely something I will be thinking about seriously.
Anyway I would be interested to know what people think about this. Is this a problem which other people have faced? How do you architect your applications to avoid this complexity?