Observable Object Proxy

By Posted on

Problem

As a part of my (future) MVVM framework, I need to be able to detect changes on an object. To do this, I used the RealProxy, a C# object that implements Aspect Oriented Programming to “intercept” calls to an object’s method.

In my scenario, I’ll use the proxy to intercept calls to a _target‘s setter to launch an event, PropertyChanged.

public delegate void PropertyChanged(PropertyChangedArgs args);

/// <summary>
/// Proxy that intercepts call to methods
/// </summary>
/// <typeparam name="T">Type of the target</typeparam>
internal class ObservableObjectProxy<T> : RealProxy where T : MarshalByRefObject
{
    private const string SetterMethodStart = "set_";
    private readonly T _target;

    public event PropertyChanged PropertyChanged;

    /// <summary>
    /// Creates a new instance of ObservableObjectProxy
    /// </summary>
    /// <param name="target">Object to proxy</param>
    public ObservableObjectProxy(T target) : base(typeof(T))
    {
        if (target == null) throw new ArgumentNullException(nameof(target));

        _target = target;
    }

    public override IMessage Invoke(IMessage msg)
    {
        var methodCall = msg as IMethodCallMessage;
        if (methodCall == null)
            throw new NotSupportedException("Invocation must be done on a method");

        var result = InvokeMethod(methodCall);

        if (!IsSetterCall(methodCall))
            return result;

        var interceptedArgs = new PropertyChangedArgs(methodCall.MethodName.Substring(4),methodCall.GetArg(0));

        PropertyChanged?.Invoke(interceptedArgs);

        return result;
    }

    private IMethodReturnMessage InvokeMethod(IMethodCallMessage callMsg)
    {
        return RemotingServices.ExecuteMessage(_target, callMsg);
    }

    private static bool IsSetterCall(IMethodCallMessage method)
    {
        return method.MethodName.StartsWith(SetterMethodStart);
    }
}

This object, as you’ve seen, is internal. To create it, I’m using a factory.

public interface IObservableObjectFactory
{
    T Wrap<T>(T obj) where T : MarshalByRefObject;
    T Create<T>() where T : MarshalByRefObject, new();
    T Create<T>(params object[] parameters) where T : MarshalByRefObject;
}

/// <summary>
/// Creates or wrap an object that will be observed by the controller.
/// </summary>
public class ObservableObjectFactory : IObservableObjectFactory
{
    private readonly IObserverController _observerController;

    /// <summary>
    /// Creates an instance of ObservableObjectFactory
    /// </summary>
    /// <param name="observerController">Controller that receives the <code>ObservableObjectProxy</code>'s PropertyChanged event. Cannot be null.</param>
    public ObservableObjectFactory(IObserverController observerController)
    {
        if (observerController == null) throw new ArgumentNullException(nameof(observerController));
        _observerController = observerController;
    }

    /// <summary>
    /// Creates a new instance of type <typeparamref name="T"/> that is observed by the controller
    /// </summary>
    /// <typeparam name="T">Type of the object to create</typeparam>
    /// <returns></returns>
    public T Create<T>() where T : MarshalByRefObject, new()
    {
        ObservableObjectProxy<T> proxy = new ObservableObjectProxy<T>(new T());
        proxy.PropertyChanged += _observerController.Handle;
        return (T)proxy.GetTransparentProxy();
    }

    /// <summary>
    /// Creates a new instance of type <typeparamref name="T"/> that is observed by the controller
    /// </summary>
    /// <typeparam name="T">Type of the object to create</typeparam>
    /// <param name="parameters">Parameters to supply to object's constructor</param>
    /// <returns></returns>
    public T Create<T>(params object[] parameters) where T : MarshalByRefObject
    {
        var obj = (T)Activator.CreateInstance(typeof(T), parameters);

        ObservableObjectProxy<T> proxy = new ObservableObjectProxy<T>(obj);
        proxy.PropertyChanged += _observerController.Handle;
        return (T)proxy.GetTransparentProxy();
    }

    /// <summary>
    /// Wraps an existing object in an <code>ObservableObjectProxy</code> that is observed by the controller.
    /// </summary>
    /// <typeparam name="T">Type of the object to wrap</typeparam>
    /// <param name="obj">Object to wrap</param>
    /// <returns></returns>
    public T Wrap<T>(T obj) where T : MarshalByRefObject
    {
        ObservableObjectProxy<T> proxy = new ObservableObjectProxy<T>(obj);
        proxy.PropertyChanged += _observerController.Handle;
        return (T)proxy.GetTransparentProxy();
    }
}

This factory is used to create or wrap an object and attach the Handle method of the controller to the created object. Here’s the controller’s interface : 

public interface IObserverController
{
    void Handle(PropertyChangedArgs args);
}

And the args for the event : 

public class PropertyChangedArgs
{
    public string PropertyName { get; set; }
    public object Value { get; set; }

    public PropertyChangedArgs(string propertyName, object value)
    {
        if (propertyName == null) throw new ArgumentNullException(nameof(propertyName));
        if (value == null) throw new ArgumentNullException(nameof(value));

        PropertyName = propertyName;
        Value = value;
    }
}

I’m looking to know if my code :

  • Makes a good use of the event pattern.
  • Makes a good use of the RealProxy.

Obviously, anything else is welcomed!

Solution

Attention Bug Alert

The way how the IsSetterCall() method is implemented is flawed. The method doesn’t clearly distinguish between a property and a method. I could call a method like

public void set_Test() { }

and your method would assume this is a property which wouldn’t cause any problems here, but later in the Invoke() method you use methodCall.GetArg(0) which would cause a TargetParameterCountException being thrown.

To be at least a little bit more on the safe side of life you can change the IsSetterCall() method like so

private static bool IsSetterCall(IMethodCallMessage method)
{
    return method.MethodBase.IsSpecialName && method.MethodName.StartsWith(SetterMethodStart);
}

but to have a real safe way you would need to add a little bit reflection voodoo like so

private static bool IsSetterCall(IMethodCallMessage method)
{
    var methodBase = method.MethodBase;
    if (methodBase.IsSpecialName && method.MethodName.StartsWith(SetterMethodStart))
    {
        var property = methodBase.DeclaringType.GetProperty(method.MethodName.Substring(SetterMethodStart.Length),
                    BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance);

        return property != null 
            && property.GetSetMethod() == methodBase as MethodInfo;
    }

    return false;
}

public class PropertyChangedArgs

The name of this class does not follow the event naming guidelines which states

Name an event argument class with the EventArgs suffix.

which means you should change the name to PropertyChangedEventArgs.

The property setters of this class should by private because you don’t want that this properties can be set from outside of the class.

Why don’t you allow null for the Value? This will restrict you from setting a property to null which is legit.

Why do you allow an empty or whitespaced string to be passed for the PropertyName? This maybe won’t happen in your current szenario but who knows what the future brings.

Another “problem” I see with the whole of your event stuff is that it doesn’t follow the typical event pattern which contains a sender.

IObserverController

It isn’t clear from this interface what it should be used for. The only hint will be that it has a Handle() method which has a PropertyChangedArgs as a parameter. One could assume that it should be used to handle the PropertyChanged event but this isn’t clear from the way the interface is named/written.

Why not simply call the interface IPropertyChangedHandler ?

Style

if (!IsSetterCall(methodCall))
    return result;

Don’t omit braces {} even when optional. Such style can lead to bugs which are hard to discover.

Although this style

if (target == null) throw new ArgumentNullException(nameof(target));

is slightly better than having the braceless instruction on the next line of code it wouldn’t hurt to use braces here as well.

Are you reinventing the wheel?

I’m sorry if this addresses just a little part of your question, but if you’re looking for a way to get an event when a property is changed, have you thought about implementing INotifyPropertyChanged (msdn)?

With this you could do stuff like:

public class Person : INotifyPropertyChanged 
{
    ...
    public string PersonName 
    {
        get { return name; }
        set { name = value; OnPropertyChanged("PersonName"); }
    }
}

And with some accompanying stuff you’ll get event handling almost for free. I’ve also seen a similar pattern, but can’t find it just now, where you set the new value as part of the property change handling.

In other words, you might have reinvented the wheel, and on the flip side I might have misunderstood your question.

A few style comments

Here goes for a more proper review of your code, not focusing on the reinvention which you seems to want to do.

  • Magic number within Invoke > var interceptedArgs = ... – You do a call to Substring(4) which presumably refers to the text after SetterMethodStart, but this should then be the length of this variable in case it got changed or someone decides to use a more common setVariableName structure in their code.
  • Is SetterMethodStart named according to conventions? – According to this you are correctly naming it like you’ve done, but as this is extremely similar to the MethodNames I’m inclined to suggesting to use SETTER_METHOD_START or _SetherMethodStart. But then again that is kind of against the guidelines…

How do you use it?!

The last point I would like to make, is that I still don’t quite get how you want to practically use this. And neither do I quite understand how you separate from get notified on all changes to any property or what/where your filter code is.

As such, there might be something said regarding useful comments within your code describing use cases and the point of using your class. The code and comments you have now are somewhat obvious and at the same time unclear as to the real purpose of your code.

A typical example is “Proxy that intercepts call to methods” which kind of says what it does, but then again why does it do that, and what is the purpose of doing this? Maybe a better comment would be “Intercepts calls to setter methods (a.k.a. methods prefixed with set_), and for these invokes PropertyChanged.Invoke with arguments`”

Leave a Reply

Your email address will not be published. Required fields are marked *