Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Enforcing the correct implementation of INotifyPropertyChanged with CodeContracts - "requires unproven"

I'm looking for an easy way to enforce the correct implementation of INotifyPropertyChanged i.e. when PropertyChanged is raised it must reference a property that is actually defined. I tried doing this with the new CodeContract tools from Microsoft, but I keep getting the warning "CodeContracts: requires unproven". Here is my code...

public sealed class MyClass : INotifyPropertyChanged
{
    private int myProperty;
    public int MyProperty
    {
        get
        {
            return myProperty;
        }
        set
        {
            if (myProperty == value)
            {
                return;
            }

            myProperty = value;
            OnPropertyChanged("MyProperty");
        }
    }

    private void OnPropertyChanged(string propertyName)
    {
        Contract.Requires(GetType().GetProperties().Any(x => x.Name == propertyName));

        var handler = PropertyChanged;
        if (handler != null)
        {
            handler(this, new PropertyChangedEventArgs(propertyName));
        }
    }

    public event PropertyChangedEventHandler PropertyChanged;
}

Is there anyway to get this to work?

like image 941
hwiechers Avatar asked Oct 25 '09 20:10

hwiechers


3 Answers

Ok, first of all, for this purpose I personally use ObservableObject implementation from the MVVM foundation. It is a DEBUG-build only runtime check almost identical to yours.

public event PropertyChangedEventHandler PropertyChanged;

protected virtual void OnPropertyChanged(string propertyName)
{
    this.VerifyPropertyName(propertyName);

    PropertyChangedEventHandler handler = this.PropertyChanged;
    if (handler != null)
    {
        var e = new PropertyChangedEventArgs(propertyName);
        handler(this, e);
    }
}

[Conditional("DEBUG")]
[DebuggerStepThrough]
public void VerifyPropertyName(string propertyName)
{
    // Verify that the property name matches a real,  
    // public, instance property on this object.
    if (TypeDescriptor.GetProperties(this)[propertyName] == null)
    {
        string msg = "Invalid property name: " + propertyName;

        if (this.ThrowOnInvalidPropertyName)
            throw new Exception(msg);
        else
            Debug.Fail(msg);
    }
}

It's probably the easiest way, but it has certain disadvantages: you need to be able to inherit from some base class, it only works in runtime (though this was always enough in my wpf-experience), it surely looks like a "patch" for a missing static check.

You have several ways to enable static analysis / static tools for this case:

  1. Like Marc says, use lambda notation and extract string in run-time.
  2. Write a custom FxCop rule.
  3. Use an AOP tool to post-process code with some meta-markup.

As for the CodeContracts, I believe it is not yet mature enough to handle this kind of checks in static analysis. Imagine, it has to parse your lambda, understand how it can be failed by a wrong propertyName, find all calls to this method, figure out all possible inputs, etc. It is just a wrong instrument for that kind of check.

like image 147
Max Galkin Avatar answered Nov 11 '22 17:11

Max Galkin


I assume you mean with the static analysis tools? (I would expect the runtime check to work, at least - and you could presumably leave it in debug builds). I doubt that this is something that static analysis is going to be able to see through - GetType().GetProperties() is simply too complex, etc.

In short; I doubt it... lambdas (Expression) are an option, but they are much slower than passing just a string.

like image 27
Marc Gravell Avatar answered Nov 11 '22 19:11

Marc Gravell


The way I have done this in the past is to use our good friend Lambda. By using Expressions we can pass in the properties themselves to your implementation of OnPropertyChanges, and use the Expression tree to extract the property. This gives you compile time checking of the members you are raising the PropertyChanged event for.

Of course use of Expression will depend entirely on what type of performance you need.

See code snippet below:

using System;
using System.Linq;
using System.ComponentModel;
using System.Linq.Expressions;

namespace OnNotifyUsingLambda
{
    public class MainClass : INotifyPropertyChanged
    {
         public static void Main (string[] args) { new MainClass().Run();}
         public void Run()
         {
              this.PropertyChanged += (sender, e) => Console.WriteLine(e.PropertyName);
              MyProperty = "Hello";
         }

         private string myProperty;
         public string MyProperty  
         {
             get
             {
                 return myProperty;
             }
             set
             {
                 myProperty = value;
                 // call our OnPropertyChanged with our lamba expression, passing ourselves.
                 // voila compile time checking that we haven't messed up!
                 OnPropertyChanged(x => x.MyProperty); 
              }
         }  

         /// <summary>
         /// Fires the PropertyChanged for a property on our class.
         /// </summary>
         /// <param name="property">
         /// A <see cref="Expression<Func<MainClass, System.Object>>"/> that contains the 
         /// property we want to raise the event for.
         /// </param>
         private void OnPropertyChanged (Expression<Func<MainClass, object>> property)
         {
             // pull out the member expression (ie mainClass.MyProperty)
             var expr = (MemberExpression)property.Body; 

             if (PropertyChanged != null)
             {
                 // Extract everything after the period, which is our property name.
                 var propName = expr.ToString ().Split (new[] { '.' })[1];
                 PropertyChanged (this, new PropertyChangedEventArgs(propName));
             }
          }

          public event PropertyChangedEventHandler PropertyChanged;
     }
}
like image 1
Mark Allanson Avatar answered Nov 11 '22 19:11

Mark Allanson