tags:

views:

333

answers:

8

When setting up some reference type properties for a project I'm working on, I came accross a few properties that needed to be properly initialized to be used and should never be null. I've seen a few ways to handle this and can't really determine if there are any major drawbacks to any of the primary ways I've seen to handle this. I'd like to get the community's opinion about the best way to handle this and what the potential drawbacks to each method might be.

Given a simple class, I've seen several ways to handle making sure a property never has a null version of this class in a property

public class MyClass
{
  //Some collection of code
}

Option 1 - Initialize the backing store

public class OtherClass1
    {
     private MyClass _mC = new MyClass();
     public MyClass MC
     {
      get { return _mC; }
      set { _mC = value; }
     }
    }

Option 2 - Initialize the property in the constructor

public class OtherClass2
    {
     public MyClass MC { get; set; }  

     public OtherClass2()
     {
      MC = new MyClass(); 
     }
    }

Option 3 - Handle initialization as needed in the Getter

public class OtherClass3
    {
     private MyClass _mC;
     public MyClass MC
     {
      get
      {
       if (_mC == null)
        _mC = new MyClass();
       return _mC; 
      }
      set { _mC = value; }
     }
    }

I'm sure there are other ways, but these are the ones that come to mind and I have seen. I'm mostly trying to determine if there's a well established best practice on this or if there's a specific concern with any of the above.

Cheers,

Steve

+4  A: 

Best option unless you really can get away with just creating a new instance yourself: only provide constructors which take all the required values, and validate them at that point.

Jon Skeet
Steve Brouillard
I'm also curious as to your opinion on using the default value attribute.
Steve Brouillard
The default value attribute is only used by the designer, as far as I'm aware. It might also be used by serialization etc - but not by normal "just create an instance" code.
Jon Skeet
It is used to optimise XML serialisation -- values that match the default value will not get serialised. As a result of the comparison performed, it's a very bad thing to have the type of the DefaultValueAttribute and the type of the property differ.
Rowland Shaw
A: 

Paraphrasing a question I posted a few days ago but I think it may be helpful to enforce code rules and make sure that Nulls a re not used where you don't want them:

Microsoft just released Code Contracts, a tool that integrates with Visual Studio and allows you to define contracts for your .Net code and get runtime and compile time checking.

Watch the video on Channel 9 that shows how it being used.

For now it's an add-on but it will be part of the Base Class Library in .Net 4.0

Renaud Bompuis
A: 

Assuming there's no side effects with regards to when _mC is instantiated, (i.e., all else being equal), I prefer option 3 as that saves the overhead of an additional instance of MyClass on the heap in the case where the getter of MC Is never called.

+2  A: 

As far as I know, there is not an established best practice here for a simple reason: each of your options has a different performance/memory footprint profile. The first option is appropriate for a reference to an object that you know must be instantiated in a class that you are sure will be used. Honestly, though, I never take this approach because I think that #2 is just more appropriate; just a sense that this is what a constructor is for.

The last option is appropriate when you are not sure whether an option will be used. It permits you take up the resource only as needed.

BTW, this question is right "next door" to a number of other issues such as the appropriate use of the Singleton pattern, the use of abstract classes or interfaces for your deferred object, etc. that might be useful for you to explore to gain greater insight.

Update: It strikes me that there is at least one case where initializing an instance in the class definition is appropriate (your Option #1). If the instance will be static then this is the only appropriate place to initialize it:

private static readonly DateTime firstClassDate = DateTime.Parse("1/1/2009 09:00:00 AM");

I thought of this when creating the above line of code in some Unit tests I was writing today (the readonly being optional wrt my point but appropriate in my case).

Mark Brittingham
A: 

Option (3) has the benefit of not allocating the object until it's needed, it can be easily adapted to be a delay loaded object (so when loading an object from a database -- keep a hold of foreign keys to load the full child object when required)

Rowland Shaw
A: 

I would use option 1, so long as it's ok for the value to remain as new MyClass(). If you require this property to be set to something meaningful, then I'd go with Jon Skeet's suggestion and add parameters to the constructor.

Jon B
A: 

options 1 & 2 are syntactically different but essentially equivalent. Option 3 is a lazy init approach that I use very consistently.
I think they all have their uses, and it depends on what you need.

Cheeso
A: 

First off should the property never be null or never null on initialisation? I suspect you meant the former in which case your setter code needs to prevent nulls being set.

The basic pattern here is that the outer class state is invalid if the inner class field has not got a valid assignment. In that case not only should the setter defend the field from null but the constructor should ensure its initialised to the correct value.

Your code implies that the outer class can instance the inner class without further input from the consuming code. In the real world the outer class needs further info from the outside in order to either receive an existing instance of the inner class or enough info for it to retrieve one.

AnthonyWJones
Thanks for the comment. You are correct in your assumption. I just threw together some sample code and didn't bother to have the setter check. In that event, however, that would have been important.
Steve Brouillard