How to choose between one big method with passive overloads and a bunch of small overloads, each one doing a small amount of work?

Question

Hi,

There are two ways to implement overloads. The first one is to do everything in one method/constructor and call it from other overloads, which leads to longer method bodies. The second one is to do the minimum in each overload, thus having a code sometimes difficult to navigate and to understand which overload does what.

For example, if two overloads of a class Cat are:

public Cat(string name, int? weight, Color mainColor);
public Cat(string name);

there are two ways to implement this:

First type

public Cat(string name, int? weight, Color mainColor)
{
    // Initialize everything.
    this.name = name;
    if (weight.HasValue) this.weight = weight.Value;

    // There is a bug here (see the anwer of @Timwi): mainColor can be null.
    this.colors = new List<Colors>(new[] { mainColor });
}

public Cat(string name)
    : this(name, null, null)
{
    // Nothing else to do: everything is done in the overload.
}

Second type

public Cat(string name)
{
    // Initialize the minimum.
    this.name = name;
    this.colors = new List<Colors>();
}

public Cat(string name, int? weight, Color mainColor)
    : this(name)
{
    // Do the remaining work, not done in the overload.
    if (weight.HasValue) this.weight = weight.Value;
    this.colors.Add(mainColor);
}

Questions

1. How those two types of overloads are called (to be able to look for further information on internet or in books)?
2. What must be the major concerns/factors to take in account when choosing between those types?

Note: since C# 4.0 let you specify optional parameters, to avoid ambiguity, let's say I'm talking about C# 3.0 only.

Answer 1

First one

Constructor overload must always call each other or a common initializer method. It is a concern of refactoring and code modularity so that you would have to make a change only once.

For example if you want to add this:

this.name = name ?? string.Empty;

In the second case you have to do it in two places for the second one but in one place in the first one.

Also per convention, order the constructors such that they start with the least parameters first.

Answer 2

I think this is another one of those examples where no single, dogmatic answer will reasonably cover all cases. I would always look at the individual case and decide based on all available factors.

One factor is that the first one has lots of ifs. Your code also has a bug: you would be adding a null value into the list of colors; in order to fix that bug, you need even more ifs. Such a constructor can easily get messy. A myriad of ifs is indicative that there are several cases in which the logic is substantially different, so having separate constructors for each of those cases makes perfect sense.

However, in cases where there aren’t that many ifs, the logic is the same for all, so now it makes sense to call a single constructor that does that one logic, and does it well. Then there’s only one place to maintain it.

Another factor is that in your example, the first one leaves weight uninitialised. This need not be a bad thing because fortunately default initialisation in C# is predictable; but I would consider it bad form if the field declaration for weight were to initialise it to something non-zero and only some of the constructors overwrite that default with another value. The constructor parameters and/or the this(...) call are better places to document the default value for that field. (Preferably the constructor parameters, because then even the client programmer can see the default value, but obviously that requires C# 4.) Of course, it is also possible to initialise all the fields using a field initialiser and leave the constructor(s) empty, which is a reasonable strategy if you only have one constructor with no arguments.

So yeah, like you said, you don’t want method bodies to get too long, but you also don’t want the code to get too difficult to navigate, so you need to strike a balance between the two for any given situation.