Refactoring large constructors

Question

We have a few objects in our domain model with what you would comically term offensively large constructors, so large that IntelliSense gives up trying to show it all to you...

Cue a type with 50 or so arguments, mostly value types, a few reference types:

public class MyLegacyType
{
    public MyLegacyType(int a1, int a2, int a3, ... int a50) // etc
    {
    }
}

I'll say it now, no this type cannot change. The type itself logically represents one entity, which happens to be property-heavy. Callers constructing this type provide the majority of the arguments from multiple sources, though some are defaulted. Perhaps there is a pattern for the sources to be provided to construction instead of the results.

However, what can change is how the type is created. Currently we have sections of code that suffer from:

• Lack of IntelliSense on the type.
• Ugly and unreadable code.
• Merging pains of due to Connascence of Position.

One immediate answer is to utilise optional parameters for default values and named arguments to help with the merging. We do this to some degree on other types, works ok.

However, it feels as though this is halfway to the full refactoring.

The other obvious solution is to reduce constructor parameters with container types that have properties for what used to be constructor arguments. This tidies the constructors nicely, and allows you to embed default values in the containers, but essentially moves the problem onto another type and possibly amounts to the same as optional / named parameter usage.

There is also the concept of Fluent constructors... both on a per property (WithIntA, WithIntB) or container type (WithTheseInts(IntContainer c)) basis. Personally, I like this approach from the calling side, but again on a large type it gets wordy and feels as though I've just moved a problem instead of solving one.

My question, if there is one buried in this mess, is: are these viable refactoring tactics for the problem? Please flesh any answer out a bit with some relevant experience, pitfalls, or criticisms. I'm leaning towards the Fluent stuff, because I think it looks cool and is quite readable and merge-friendly.

I feel as though I'm missing the Holy Grail of constructor refactorings - so I'm open to suggestions. Of course, this could also just be an unfortunate and unavoidable side effect of having a type with this many properties in the first place...

Answer 1

Obviously we don't have much context here, but at 50+ parameters my interpretation is that this class is doing too much, and is too complex. I would start by looking for ways to split chunks out into simpler, more focused types - and then encapsulate instances of each of those concepts into the composite class. So it becomes:

public MyLegacyType(SomeConcept foo, AnotherConcept bar, ...)
{
}

where only the logic necessary for orchestrating between the concepts remains in MyLegacyType (any logic particular to SomeConcept goes there, etc).

This is different to your "reduce constructor parameters with container types that have properties for what used to be constructor arguments", since we are fundamentally restructuring the logic - not just just using an object to replace the constructor arguments.

Answer 2

I would go with the container types and use the immediate properties assignment of C# 4.0. This way one could easily use Intellisense on the resulting type, while still retaining a decent decoupling from the original type.

For example:

public class MyLegacyType
{
    public MyLegacyType(MyConfiguration configuration) // etc
    {
      // ...
    }
}

public class MyConfiguration
{
   public int Value1 { get; set; }
   public int Value2 { get; set; }
   // ...
}

And then:

var myInstance = new MyLegacyType(new MyConfiguration
{
  Value1 = 123,
  Value2 = 456
});

Answer 3

There's one thing that I'm not sure about your question, and that is why do you want all such parameters in the constructor? Are you using all of the parameters in the constructor code? Your problem with the intellisense is probably coming from having too many parameters on a single method. Having many number of fields / properties on a single type won't cause any issues.

It seems that you've seen some ways to manage the number of args, but if you can explain why you need to receive all of them in a constructor, we can think outside this box. There might be something there to look at.