How to refactor a class with overloaded constructors

Question

I have a class with overloaded constructor (C#) It can be initialized in few ways, and some parameters are optional - so in result - there is a confusing bunch of constructors

new Object(StrA, StrB, ObjA)
new Object(StrA, StgB, ObjB, StrC)
new Object(StrA, StrB, ObjA, StrD)
new Object(StrA, StrB, ObjB, StrC, StrD)
new Object(StrA, StrB, StrE, ObjA)
new Object(StrA, StrB, StrE, ObjB)
new Object(StrA, StrB, StrE, ObjA, StrC)
new Object(StrA, StrB, StrE, ObjB, StrC, StrD)

I see a two ways to improve situation a) create a structure to hold optional parameters

new Config(StrA, StrB, StrD, StrE) 
new Object(Config, ObjA)
new Object(Config, ObjB, StrC)

b) set optional parameters as properties

A = new Object(ObjA)
A.StrA = some;
A.StrB = some;
A.StrD = some;
A.StrE = some;

Which is the best way?

And is such code refactoring necessary - the previous code maintainer says that "while intellisense used, complexity of constructor doesn't matter - it always possible to check tips and select the correct one"

Answer 1

Neither one is any better than the other; in fact you could use a combination of both depending upon the types and usages of your parameter. Personally, I prefer to use more properties (thus option B), but with C# 4.0, we'll have optional parameters built-in! You might just want to wait until then.

Answer 2

Another option is to hold on for the moment, and then compress it all down when C# 4 comes out with optional parameters and named arguments - so long as you don't need to call the code from any other languages which might not have those features.

I'd personally create a type holding all the parameters, unless there are different types involved for some overloads (e.g. like XmlReader.Create can deal with a TextReader or a Stream). I wouldn't make it a struct though - just make it a class, it'll be fine. Give it a load of automatically implemented properties, and then you can use object initializers to set the parameters in a readable way.

Answer 3

If they're optional, they probably don't need to be set inside the constructor (probably). In which case you could remove all but the simple constructors, and call them as

new Object(ParamA) {PropB = ParamB, PropC = ParamC, PropD = ParamD}

This can only be used to set public, settable properties, but if that's what those parameters correspond to, it's a better solution in my opinion. I see parameters to the constructor as being things which are either a) necessary for proper construction of the object, or b) unchangeable after the fact. Anything else should be set as a property.

Answer 4

When constructing an object becomes complex for whatever reason, there're always the option of the Factory Pattern

In this case using the factory method pattern is somewhat like option b)

var factory = new ObjFactory();
factory.StrA = some;
factory.StrB = some;

var obj = factory.Create();

But doing it like this has the benefits that, if the constructor were ever to be changed again, all you need to do is to change the factory implementation. The code that depends on the object implementation does not need to change much if it needed to be changed at all.

Changing the way dependencies are injected in the class are not always a good option. Especially when you are working against a legacy codebase.

If you used option b) that would means that all methods on the object should check for the proper combination of assigned property before being invoked or else weird things might happens. But if you used the factory pattern, you can still keep the old constructor and the class implementations without introducing new code into the object.

Answer 5

Yes, I would refactor this. IntelliSense is only so much helpful, and staring at a 5-argument constructor which takes 3 seemingly random strings trying to figure out which one means what is not going to improve code readability any. In VB (or C# 4.0), I'd still go with constructor, but use named arguments. For C# 3.0, I'd make a separate class to hold the initialization info:

class ObjectSettings
{
    public string StrA { get; set; }
    public string StrB { get; set; }
    ...
    public string ObjA { get; set; }
    public string ObjB { get; set; }
}

and then take a single argument of that type in constructor:

class Object
{
    public Object(ObjectSettings settings);
}

and use object initializer when invoking it:

new Object(new ObjectSettings { StrA = ..., StrB = ..., ObjA = ... })

The main advantage of this compared to just having properties in Object is that this pattern guarantees that Object will be properly initialized as soon as it is constructed. With properties on object itself, it will be in invalid state until client sets them all correctly, and you'll have to validate that on pretty much every call.

This pattern is actually used in .NET FCL - see XmlReader / XmlReaderSettings for an example of it.

Answer 6

I would make the optional parameters simple properties. The type initializer syntax in C# 3.5 makes this very readable, e.g.:

A = new Object(ObjA)
{
    StrA = someA,
    StrB = someB,
    StrD = someD,
    StrE = someE
}

Answer 7

Don't know if it's relevant to your specific situation, but... Sometimes when I've been faced with a large number of constructors of slightly different flavours the result has been that the class being constructed was trying to do too much. Creating subclasses for different cases meant less constructors per class and a clearer definition of what was being constructed. This is most relevant if some of the parameters are entirely optional, or only make sense if others are also set.

Answer 8

I would consider refactoring to Introduce Parameter Object or using a Factory.

If a class' constructors become complex, then it breaks the single responsibility principle, in that the object has the responsibility to create itself correctly and to do the work it is designed to do.

Answer 9

If you move a constructor argument to being a property, bear in mind that this will allow the consumer of your object to change that property at any point. This might not always be possible, depending on how your object works.

Having said that, I tend to prefer properties for parameters that are genuinely optional, or can be given sensible default values. This tends to make your object much simpler to use for the "normal" case.

Answer 10

A: it depends.

Why use parameters over properties over config? ...It depends.
How cohesive are the args?
Do they have reasonable defaults?
Do they depend on each other?...etc...

Ultimately, what is the most simplest/complete/appropriate expectation for a user of your ctor()?