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

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:

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.
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.
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.

The best way would be to find ways to group the arguments together. This assumes, and really only works if, you would end up with multiple "groupings" of arguments.

For instance, if you are passing the specification for a rectangle, you can pass x, y, width, and height or you could just pass a rectangle object that contains x, y, width, and height.

Look for things like this when refactoring to clean it up somewhat. If the arguments really can't be combined, start looking at whether you have a violation of the Single Responsibility Principle.

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

Related

Recent Posts