INotifyPropertyChanged & DynamicProxy: improvements

I have talked before about INotifyPropertyChanged and a simple implementation with DynamicPorxy. This time I will show you some enhancements.

This is my IInterceptor (Castle Dynamic Proxy):

using System;
using System.Collections.Generic;
using System.ComponentModel;
using Castle.Core.Interceptor;

namespace uNhAddIns.ComponentBehaviors.Castle
{
    public class PropertyChangedInterceptor : IInterceptor
    {
        private PropertyChangedEventHandler _handler;

        #region IInterceptor Members

        public void Intercept(IInvocation invocation)
        {
            string methodName = invocation.Method.Name;

            if (invocation.Method.DeclaringType.Equals(typeof (INotifyPropertyChanged)))
            {
                if (methodName == "add_PropertyChanged")
                    _handler = 
                        (PropertyChangedEventHandler) Delegate.Combine(_handler, 
                                                     (Delegate) invocation.Arguments[0]);
                if (methodName == "remove_PropertyChanged")
                     _handler = (PropertyChangedEventHandler) Delegate.Remove(_handler, 
                                                     (Delegate) invocation.Arguments[0]);

                return;
            }
            
            invocation.Proceed();

            if (methodName.StartsWith("set_"))
                NotifyPropertyChanged(methodName, invocation.Proxy);
        }

        #endregion

        protected void NotifyPropertyChanged(string methodName, object proxy)
        {
            string propertyName = methodName.Substring(4);
            var args = new PropertyChangedEventArgs(propertyName);
            OnPropertyChanged(proxy, args);
        }

        protected void OnPropertyChanged(Object sender, PropertyChangedEventArgs e)
        {
            PropertyChangedEventHandler eventHandler = _handler;
            if (eventHandler != null) eventHandler(sender, e);
        }
    }
}

Problem: don’t notify when property doesn’t change

You don’t have to notify every time, only when the property really change, otherwise event-subscribers will have to do unnecessary things. So I will add the following test to my test suite:

[Test]
public void should_not_raise_propertychanged_when_value_doesnt_change()
{
    bool eventWasRaised = false;

    var album = container.Resolve<Album>();
    album.Title = "dark side";

    ((INotifyPropertyChanged)album).PropertyChanged +=
        (sender, e) => eventWasRaised = true;;

    album.Title = "dark side";
    eventWasRaised.Should().Be.False();
}

This test is very easy to satisfy, we need to grab the old value before proceed the invocation.
This is how we treat a set_Property now:

//remove the "set_" from the method name (for isntance set_Title)
string propertyName = invocation.Method.Name.Substring(4);

//The lastvalue is the NEW val. 
//This is important when you are setting INDEXED properties.
var indexes = invocation.Arguments
              .Where((obj, index) => 
                     index < invocation.Arguments.Length - 1)
              .ToArray();

//store the current value.
var oldValue = GetCurrentValueForProperty(invocation.Proxy,
                                          propertyName, indexes);

//store the new value
var newValue = invocation.Arguments.Last();

//proceed with the set
invocation.Proceed();

//if the value has changed, notify
if (oldValue != newValue)
    OnPropertyChanged(invocation.Proxy, propertyName);

 

Problem: readonly properties

This problem is more complicated. Suppose you have the following class:

public class Person
{
        public virtual string FirstName { get; set; }
        public virtual string LastName { get; set; }
        public virtual string Address { get; set; }
        public virtual string FullName
        {
            get
            {
                return FirstName + ' ' + LastName;
            }
        }
}

Changing “FirstName” should raise a notification for FirstName property, the same with LastName and Address. But, when we should notify that FullName has changed?. The answer is simple whenever First or Last really change.

The call to a set property does not imply that the value has actually changed.

The test:

[Test]
public void can_raise_property_changed_for_readonly_property()
{
    var person = container.Resolve<Person1>();
    bool eventWasRaised = false;

    ((INotifyPropertyChanged)person).PropertyChanged +=
            (sender, e) =>
            {
                if(e.PropertyName == "FullName")
                {
                    person.FullName.Should().Be.EqualTo("Jose");
                    eventWasRaised = true;
                }
                    
            };

    person.FirstName = "Jose";
    eventWasRaised.Should().Be.True();
}

Not only we are testing that the event is raised but also that is raised after change the FirstName value.
I have updated the IInterceptor as follows:

private IDictionary<string, PropertyInfo> _readOnlyProperties;
private IDictionary<string, object> _readOnlyPropertiesValues; 

public void Intercept(IInvocation invocation)
{
    //..

    //FIRST TIME - FIRST SET we grab readonly properties with values.
    if (_readOnlyProperties == null)
        GrabReadOnlyProperties(invocation.Proxy);

    //proceed with the set
    invocation.Proceed();

    //if the value has changed, notify
    if (oldValue != newValue)
        OnPropertyChanged(invocation.Proxy, propertyName);

    NotifyReadOnlyPropertiesChanges(invocation.Proxy);
}

private void NotifyReadOnlyPropertiesChanges(object proxy)
{
    foreach (var property in _readOnlyProperties)
    {
        object oldValue = _readOnlyPropertiesValues[property.Key];
        object newValue = property.Value.GetValue(proxy, null);
        if(oldValue != newValue)
        {
            _readOnlyPropertiesValues[property.Key] = newValue;
            OnPropertyChanged(proxy, property.Key);
        }
    }
}

//this method should be called ONCE.
private void GrabReadOnlyProperties(object proxy)
{
    _readOnlyProperties = proxy.GetType().GetProperties()
                               .Where(p => p.CanWrite == false)
                               .ToDictionary(p => p.Name, p => p);

    _readOnlyPropertiesValues = _readOnlyProperties
                                .ToDictionary(p => p.Key,
                                   p => p.Value.GetValue(proxy, null));
}

Problems:

  • I’m using too much reflection and this will affect the performance.
  • There are other cases where I can not intercept a change in read only properties. For instance: Invoice.Total, depends upon Invoice.Lines.UnitPrice – Invoice.Lines.Quantity.

This work is part of the uNhAddIns.ComponentBehaviors.


blog comments powered by Disqus
  • Categories

  • Archives