refactoring - What's the best way to refactor a method that has too many (6+) parameters?

Question

Occasionally I come across methods with an uncomfortable number of parameters. More often than not, they seem to be constructors. It seems like there ought to be a better way, but I can't see what it is.

return new Shniz(foo, bar, baz, quux, fred, wilma, barney, dino, donkey)

I've thought of using structs to represent the list of parameters, but that just seems to shift the problem from one place to another, and create another type in the process.

ShnizArgs args = new ShnizArgs(foo, bar, baz, quux, fred, wilma, barney, dino, donkey)
return new Shniz(args);

So that doesn't seem like an improvement. So what is the best approach?

Answer 1

I'm going to assume you mean C#. Some of these things apply to other languages, too.

You have several options:

switch from constructor to property setters. This can make code more readable, because it's obvious to the reader which value corresponds to which parameters. Object Initializer syntax makes this look nice. It's also simple to implement, since you can just use auto-generated properties and skip writing the constructors.

class C
{
    public string S { get; set; }
    public int I { get; set; }
}

new C { S = "hi", I = 3 };

However, you lose immutability, and you lose the ability to ensure that the required values are set before using the object at compile time.

Builder Pattern.

Think about the relationship between string and StringBuilder. You can get this for your own classes. I like to implement it as a nested class, so class C has related class C.Builder. I also like a fluent interface on the builder. Done right, you can get syntax like this:

C c = new C.Builder()
    .SetX(4)    // SetX is the fluent equivalent to a property setter
    .SetY("hello")
    .ToC();     // ToC is the builder pattern analog to ToString()

// Modify without breaking immutability
c = c.ToBuilder().SetX(2).ToC();

// Still useful to have a traditional ctor:
c = new C(1, "...");

// And object initializer syntax is still available:
c = new C.Builder { X = 4, Y = "boing" }.ToC();

I have a PowerShell script that lets me generate the builder code to do all this, where the input looks like:

class C {
    field I X
    field string Y
}

So I can generate at compile time. partial classes let me extend both the main class and the builder without modifying the generated code.

"Introduce Parameter Object" refactoring. See the Refactoring Catalog. The idea is that you take some of the parameters you're passing and put them in to a new type, and then pass an instance of that type instead. If you do this without thinking, you will end up back where you started:

new C(a, b, c, d);

becomes

new C(new D(a, b, c, d));

However, this approach has the greatest potential to make a positive impact on your code. So, continue by following these steps:

1. Look for subsets of parameters that make sense together. Just mindlessly grouping all parameters of a function together doesn't get you much; the goal is to have groupings that make sense. You'll know you got it right when the name of the new type is obvious.

2. Look for other places where these values are used together, and use the new type there, too. Chances are, when you've found a good new type for a set of values that you already use all over the place, that new type will make sense in all those places, too.

3. Look for functionality that is in the existing code, but belongs on the new type.

For example, maybe you see some code that looks like:

bool SpeedIsAcceptable(int minSpeed, int maxSpeed, int currentSpeed)
{
    return currentSpeed >= minSpeed & currentSpeed < maxSpeed;
}

You could take the minSpeed and maxSpeed parameters and put them in a new type:

class SpeedRange
{
   public int Min;
   public int Max;
}

bool SpeedIsAcceptable(SpeedRange sr, int currentSpeed)
{
    return currentSpeed >= sr.Min & currentSpeed < sr.Max;
}

This is better, but to really take advantage of the new type, move the comparisons into the new type:

class SpeedRange
{
   public int Min;
   public int Max;

   bool Contains(int speed)
   {
       return speed >= min & speed < Max;
   }
}

bool SpeedIsAcceptable(SpeedRange sr, int currentSpeed)
{
    return sr.Contains(currentSpeed);
}

And now we're getting somewhere: the implementation of SpeedIsAcceptable() now says what you mean, and you have a useful, reusable class. (The next obvious step is to make SpeedRange in to Range<Speed>.)

As you can see, Introduce Parameter Object was a good start, but its real value was that it helped us discover a useful type that has been missing from our model.