Fields and virtual members


It seems like the sillier the bug, the more time you’ll spend debugging it, simply because it’s in functionality you just knew that worked correctly (and had the tests to back it up).  One problem we hit recently was code that used fields to back virtual properties:

public abstract class Enumeration : IComparable
{
    private readonly int _value;

    protected Enumeration() { }

    protected Enumeration(int value)
    {
        _value = value;
    }

    public virtual int Value
    {
        get { return _value; }
    }

    public virtual int CompareTo(object other)
    {
        return _value.CompareTo(((Enumeration)other).Value);
    }
}

This isn’t a problem in and of itself, except in the behavior of that last method.  Notice its logic uses the field and not the property.  I can then define a derived type:

public class PercentDiscount : Enumeration
{
    public override int Value
    {
        get { return 5; }
    }
}

But now my CompareTo method breaks because it isn’t using the correct value.  It uses the uninitialized field, causing all sorts of bizarre behavior.  When you have logic errors in places like GetHashCode, Equals or CompareTo, I can guarantee your application will break in the most hair-pulling ways imaginable.

Let’s fix it

So what’s a better option?  We could just “make sure” that we don’t use the private field, and only use the virtual property.  But I hate conventions that rely on my memory to not make a mistake, I want to fall into the pit of success.  One way to do so is to use automatic properties:

public abstract class Enumeration : IComparable
{
    protected Enumeration() { }

    protected Enumeration(int value)
    {
        Value = value;
    }

    public virtual int Value
    { 
        get;
        private set;
    }

    public virtual int CompareTo(object other)
    {
        return Value.CompareTo(((Enumeration)other).Value);
    }
}

Instead of a backing field, we’ll use an automatic property, making it impossible to use a non-virtual backing field.  We still get the same access modifiers (a virtual, overridable getter, a private setter), plus the benefits of automatic properties.  So now I’m curious: are there any valid reasons to keep using fields for holding state?

MVC Best Practices