Is nesting constructors (or factory methods) good, or should each do all init work

Question

Is it a good idea (from a design POV) to nest constructor calls for overloaded New or Factory style methods? This is mostly for simple constructors, where each overload builds on the previous one.

MyClass( arg1 ) { 
    _arg1 = arg1; 
    _otherField = true; 
    _color="Blue" 
}
MyClass( arg1, arg2) : this(arg1) { 
    _arg2 = arg2  
}
MyClass( arg1, arg2, arg3) : this(arg1, ar2) { 
    _arg3 = arg3; 
}

Or with factory methods:

static NewInstance(arg1 ) { 
   _arg1 = arg1;       
}
static NewInstance(arg1, arg2) {
   f = NewInstance(arg1);
   f._arg2 = arg2;
}
//... and so on

I can see a few drawbacks on both sides

• Nesting hides what the constructor is doing
• Not nesting duplicates all the functionality

So, is doing this a good idea, or does it set me up for something I'm just not seeing as a problem. For some reason I feel uneasy doing it, mostly because it divides up the responsibility for initializing.

Edit: @Jon Skeet: I see now why this was bothering me so much. I was doing it backwards! I wrote the whole thing and didn't even notice, it just smelled. Most other cases I have (that I wrote), do it the way you recommend, but this certainly isn't the only one that I have done like this. I do notice that the more complicated ones I did properly, but the simple ones I seem to have gone sloppy. I love micro edits. I also like acronymns!

Answer 1

I think it's reasonable to chain constructors together, but I do it the other way - the version with fewer parameters calls the version with more parameters. That way it makes it very clear what's happening, and all the real "logic" (beyond the default values) is in a single place. For example:

public Foo(int x, int y)
{
    this.x = x;
    this.y = y;
    precomputedValue = x * y;
}

private static int DefaultY
{
    get { return DateTime.Now.Minute; }
}

public Foo(int x) : this(x, DefaultY)
{
}

public Foo() : this(1, DefaultY)
{
}

Note that if you have lots of constructor overloads, you may wish to move to static factory methods instead - that usually makes the code clearer, as well as allowing multiple methods to take the same set of parameters, e.g.

public static XmlDocument FromText(string xml)

public static XmlDocument FromFile(string filename)

Answer 2

If you're developing in .Net 3.5 I recommend never using constructors. The only exception I leave for this is if you are using an injection constructor for dependency injection. With .Net 3.5 they created object initializers which allow you to do

var myclass = New MyClass { arg1 = "lala", arg2 ="foo" }

This will initalize the class with values assigned for arg1 and arg2 while leaving arg3 as

default(typeof(arg3)).