A few time ago, one of my colleague has seen the following code
class UsingVariable
{
private int myValue;
public int MyValue
{
get { return myValue; }
set { myValue = value; }
}
public UsingVariable()
{
myValue = 10;
}
}
and proposed us to migrate it to that
class UsingAutoImplementedProperties
{
public int MyValue { get; set; }
public UsingAutoImplementedProperties()
{
MyValue = 10;
}
}
argueing /explaining that now with the .NET 3.5 new features, it was possible, and as the local variable was not usefull, we could get rid of it.
When he told that I had a strongly opposed reaction. We should not do that. The local variable is not usefull ? And what about if the property was harmfull ? Let's see what could happen.
Comparison of the two codes:
Well nothing really to see. The code are currently perfectly equivalent. They will behave the same. Some purist may tell that the second is "slightly" slower (as using a call to property, ie to a method). Well... This is perfectly negligeable.
So these two codes are equivalent. I would precise : these two codes are equivalent... at the condition you assume your code will never get touched and changed in the future. And I'm not sure this is a bet an english bookmaker would do.
Let's see two possible evolutions and check what would happen.
Evolution 1 : some event raising in the property
You all know that for an auto-implemented property, both the get AND the set must be auto-implemented. As a consequence, if you need / want to do some extra treatment in one of the two, you will need to get rid of the auto-implementation to do it yourself. What could happen ? Let's imagine something like :
class UsingAutoImplementedProperties_Evolution1
{
private int myValue;
public int MyValue
{
get { return myValue; }
set
{
if ( myValue != value )
{
OnMyValueChanging(EventArgs.Empty);
myValue = value;
OnMyValueChanged(EventArgs.Empty);
}
}
}
}
We'll simply follow some .NET conventions to raise an event when the property has been changed. Well is that impossible ? Far from that I think.
What may happen ? Well, an event may be raised to indicate to a client that the internal state of the object has been changed. And the client may do some treatment on the object whose internal state has changed. Well yes but on which object really ? On that object I'm manipulating and on which I'm still not finished with the constructor ? Meaning on that object that may have any incorrect or un-initialized values ?
We can just smell there is something going wrong here.
Note that this case may not happen in real life. If I want to handle the event, I will need to have an instance no ? Right. Anyway, this example - not realistic but not so far from real life - let us smell and glimpse there may be some tricky problem to find out there.
Evolution 2 : Adding a level of hierarchy
Let's imagine we need to add a level of hierarchy in our class. We'll simply put for example the property as virtual.
class UsingAutoImplementedProperties_Evolution2
{
public virtual int MyValue { get; set; }
public UsingAutoImplementedProperties_Evolution2()
{
MyValue = 10;
}
}
And let's create any derived class, let's imagine like this.
class DerivedClass : UsingAutoImplementedProperties_Evolution2
{
private OtherClass objectInitializedInCtor;
public DerivedClass()
{
objectInitializedInCtor = new OtherClass();
}
public override int MyValue
{
get { return base.MyValue; }
set
{
objectInitializedInCtor.DoSomething();
base.MyValue = value;
}
}
}
What is the consequence ? When I will do a new DerivedClass(), here is the workflow that will occur (I will simplify it by giving only the steps related to the code example) :
-
Initializing the variables of the class
-
Calling the constructor of the class
-
Calling the need for building the base class
-
Initializing the variables of the base class
-
Calling the constructor of the class
-
Calling the property MyValue to set 10
-
This will in fine call the property MyValue of the derived class
-
This will call the method DoSomething on the objectInitializedInCtor (which is null)
-
This will crash
This case is pretty simple as it will crash. Let's imagine now we are dealing with a collection that has been created, but not yet filled in, or on some values that still have their default value instead of the real one, this may cause very tricky bugs to find out.
Is that bad ? It's not my code...
What can we see here ? The application may crash / bug not because of the original code, but because of the evolutions we may bring to this code. The question may be "who is responsible for the bug ?".
Well in my view, the responsible is for sure the first code and NOT the evolution. Indeed we have assumed that our code WILL NOT evolve, and so we have put in place un-needed risks. And the guy making the evolution has just made a simple evolution, touching simply what he would need, and not more. I would even say, just what he was supposed to touch.
Imagining that the original code will not take this risk, is that defensive programmation ? Not in my view. For me defensive programmation is putting many things in place, because we thing the next one will add some bad / buggy code. Here it's the contrary, it's just writing the code to avoid having the next developper needing to think the ones before has written some bad / buggy code.
Does this resolve everything ?
No of course. But before detailing any other aspects related to the risky original code, or to the proposed code (ie without using properties in the constructor), I would like to listen to the reactions of all of you. To know what you think on the subject and which limitations you see.