Los Techies : Blogs about software and anything tech!

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?

Kick It on DotNetKicks.com
Posted Nov 09 2009, 09:40 AM by bogardj
Filed under:

Comments

Akash Chopra wrote re: Fields and virtual members
on 11-09-2009 10:38 AM

You still need fields if you want them to use the readonly modifier :(

Erik wrote re: Fields and virtual members
on 11-09-2009 10:53 AM

You also need backing fields if you intend to do something with the property value inside the getter or setter. It would be *so* nice to be able to use the 'value' keyword in this way, both in the getter and setter...

Frank Quednau wrote re: Fields and virtual members
on 11-09-2009 11:10 AM

With wholly unexposed state I have no quarrel with fields. I also like using the readonly modifier for state that is immutable after instance creation

Rob wrote re: Fields and virtual members
on 11-09-2009 12:35 PM

This is why I'd like to see a form of property that has the backing field within the property description like so:

public int MyProp

{

int _myProp;

get { return _myProp; }

set { _myProp = value; }

}

That way you could still do all the custom get/set code that you want without exposing the backing field.

An alternative would be to add a new keyword like "value" to represent the auto created backing store value.

bogardj wrote re: Fields and virtual members
on 11-09-2009 2:32 PM

@Akash, Frank

Yah, I forgot about the readonly modifier.  Though, I typically only use that for services (dependencies), collection types, or immutable objects.

@Rob et al

I see properties with logic as a bad idea.  Methods are more expressive that extra things are happening when getting or setting values.

Paco wrote re: Fields and virtual members
on 11-09-2009 2:32 PM

Now you have a bug in the constructor of an inheritor when you override the virtual method (and break the LSP)

AlexEzh wrote re: Fields and virtual members
on 11-09-2009 3:38 PM

Why not set _value to 5 which would keep the existing class logic? I understand that the class is an example, but you are trying to hide the behavior of the base class which is a violation of Liskov substitution principle. Article on Wikipedia has a good example.

Christopher Harris wrote re: Fields and virtual members
on 11-09-2009 3:48 PM

@Paco: I have to admit you make a very interesting observation on breaking the LSP.  It makes me wonder if this is one of those cases where a new trivial unit test would have been more appropriate to catch the CompareTo bug. (Clean Code: Smell T3: Don't Skip Trivial Tests)

bogardj wrote re: Fields and virtual members
on 11-09-2009 3:57 PM

@Paco et al

LSP isn't violated here.  Fields are an implementation detail and should never be exposed outside of that class (i.e., instance fields should always be private).

It's just one of those cases where if a class exposes state, it should be self-consistent and use the property accessors.

AlexEzh wrote re: Fields and virtual members
on 11-09-2009 5:29 PM

I would say differently. It is fine to use fields in the class implementation and expose property accessors as long as accessors are not virtual. If virtual accessor is required, than ideally it should be abstract to avoid unnecessary fields in the class implementation.

Artem Govorov wrote re: Fields and virtual members
on 11-10-2009 12:24 AM

Presentation frameworks (WinForms, WPF) UI elements 2-way data binding heavily depends on read/write properties and INotifyPropertyChanged interface implementation, so property setter is an obvious place to put code raising property changed event. I'm not saying that this is good or bad - it just the way it is and this makes it impossible to use auto properties in these cases.

Reflective Perspective - Chris Alcock » The Morning Brew #474 wrote Reflective Perspective - Chris Alcock » The Morning Brew #474
on 11-10-2009 3:35 AM

Pingback from  Reflective Perspective - Chris Alcock  » The Morning Brew #474

Fizz wrote re: Fields and virtual members
on 11-10-2009 4:38 AM

I want Property scoped values, solves all of these problems

public int MyProperty

{

   int myField;

  get { return myField; }

  set

  {

       if (!MyFieldValid(value) throw new Exception("");

       myField = value;

       NotifyPropertyChanged("MyProperty");

   }

}

MS does not think this is usefull

Fields and virtual members « Jasper Blog wrote Fields and virtual members « Jasper Blog
on 11-10-2009 9:23 AM

Pingback from  Fields and virtual members « Jasper Blog

Ryan R wrote re: Fields and virtual members
on 11-10-2009 3:55 PM

How about lazy loading? I have yet to see a good way (no reflection) of lazy loading an automatic property.

bogardj wrote re: Fields and virtual members
on 11-11-2009 8:10 AM

@Ryan

Do you mean manual lazy loading?  I haven't done that kind of code in a long, long time.

Mahesh wrote re: Fields and virtual members
on 11-13-2009 9:50 AM

Since property is converted into method in IL, there could be slight performance hit as it is method call.

Dan Puzey wrote re: Fields and virtual members
on 11-16-2009 2:15 PM

While I agree that you shouldn't have "behaviour" in a property, it's not difficult to think of cases where you need logic in a property: I'd expect most setters to have some basic validation in them - checking for null, or negative numbers or whatever.  That typically requires a backing field, unless there's some syntax I haven't learnt yet (which is entirely possible :-))

Add a Comment

(required)  
(optional)
(required)  
Remember Me?

Enter the numbers above:
Copyright Los Techies 2008, 2009. All rights reserved.
Powered by Community Server (Commercial Edition), by Telligent Systems