When dealing with toy examples, it can be difficult to see the value in “best practices.” In such small and isolated environments, it’s hard to see the merits of the GOOD and the BAD.

One such smell that I had trouble understanding was the Control Parameter smell. This usually manifests itself something like the following:

def print_address(include_zip_code)
  if include_zip_code
    street_1 + city + state + zip_code
  else
    street_1 + city + state
  end
end

The problem with the include_zip_code parameter is that it doesn’t reflect any new knowledge. The caller already knows which path should be taken, i.e. whether it wants the ZIP code or not.

These methods also tend to take in boolean parameters, which are difficult to decipher without code completion in your IDE:

print_address(true) # What does this `true` mean again?

This post will cover how to untangle tightly-coupled control parameters. I particularly like this approach because it sets code modules on a healthy growth trajectory. This approach paves a path for objects to grow and change over time in way that won’t make you go cross-eyed.

As always, the examples are in Ruby but can applied to just about any language.

Bigger and Badder

We recently completed a refactoring at Gusto that included ripping out many of these control parameters. Our starting point looked something like the following:

class PayrollCalculator
  def initialize(payroll, options = {})
    @payroll = payroll
    @skip_benefits = options[:skip_benefits]
    @skip_taxes = options[:skip_taxes]
    @skip_donations = options[:skip_donations]
  end

  def calculate
    if !@skip_benefits
      result = calculate_benefits(@payroll)
    end

    if !@skip_donations
      result = calculate_donations(@payroll, result)
    end

    if !@skip_taxes
      result = calculate_taxes(@payroll, result)
    end

    result
  end
end

The actual code was more convoluted than this, spanning thousands of lines with the conditional forks splintered across many different methods and classes. You can bet your butt that there was more than a single line within each conditional.

If we count the contexts, you’ll notice that there are at least 23 ways through this method. In this example, this class is also doing much more than just calculating: it’s coordinating. Usually I’ll try to keep these two concepts separate.

If we visualize this class, it looks like the following:

A Walk Through Our Control Parameter Class

By just sketching out the behavior of the class, we can see it’s doing quite a bit.

Strategy and Steps

Our strategy to fix this will be to push up the conditional logic by removing the control parameters.

We want to push out the conditional logic to simplify the execution of the class in question. By simplifying the module in question, it will become easier to reason about, maintain, and test.

We want to make the execution look like this:

To get there, we will perform the following steps:

  1. Backfill any tests for what we’re about to change
  2. Identify all control parameters
  3. Decide whether to work top-to-bottom or bottom-to-top
  4. Choose the first/last instance of the control parameter
  5. Extract the “guts” of the logic being conditionalized into one or more service objects, preferably PFaaOs. Create a class for each conditional branch.
  6. Create a null service object for the “empty” conditional case
  7. Inject the new class as a default parameter into the class
  8. Convert all callers to select which service class they wish to use
  9. Repeat until we’ve removed all control parameters

This will be made easier if:

  • Our class or method treats data immutably; rather than modifying data in place it will create a new copy of the data. Immutable data helps define our interfaces a bit more crisply.
  • Our methods are pure functions, meaning they incur no side effects.
  • The code to be refactored is (temporarily) moved into one large method.

The First Step

For this example, we’ll work top-to-bottom beginning first with our benefits calculation logic above.

First, we want to extract the guts of the logic into a service class (Step 5). Because we are dealing with immutable data and pure functions, the substitution is simple:

class PayrollCalculator
  def initialize(payroll, options = {})
    @payroll = payroll
    @skip_benefits = options[:skip_benefits]
    @skip_taxes = options[:skip_taxes]
    @skip_donations = options[:skip_donations]
  end

  def calculate
    if !@skip_benefits
      result = BenefitsCalculator.calculate(@payroll)
    end

    if !@skip_donations
      result = calculate_donations(@payroll, result)
    end

    if !@skip_taxes
      result = calculate_taxes(@payroll, result)
    end

    result
  end
end

class BenefitsCalculator
  def self.calculate(payroll)
    # The guts of the method formerly known
    # as PayrollCalculator#calculate_benefits
  end
end

Here we’ve extracted some of the logic into a service class. This hasn’t done much for the moment other than modularize our code a bit more.

Pull Up the Service Class

Next, we need to pull up the usage of the service class higher into the operation of the PayrollCalculator. To do so, we’ll move it to the constructor:

class PayrollCalculator
  def initialize(payroll, options = {})
    @payroll = payroll

    if options[:skip_benefits]
      @benefits_calculating_klass = NullCalculator
    else
      @benefits_calculating_klass = BenefitsCalculator
    end

    @skip_taxes = options[:skip_taxes]
    @skip_donations = options[:skip_donations]
  end

  def calculate
    result = @benefits_calculating_klass.calculate(@payroll)

    if !@skip_donations
      result = calculate_donations(@payroll, result)
    end

    if !@skip_taxes
      result = calculate_taxes(@payroll, result)
    end

    result
  end
end

class BenefitsCalculator
  def self.calculate(payroll)
    # The guts of the method formerly known
    # as PayrollCalculator#calculate_benefits
  end
end

class NullCalculator
  def self.calculator(payroll, result)
    result
  end
end

If we look at our #calculate method, we now notice that the BenefitsCalculator is nowhere to be found. Its reference has moved up into the constructor of the class.

Importantly, we also have performed Step 6 and have created a “null” service object. This is an implementation of the Null object pattern. Its job is to do “nothing.” In our case, that means performing no calculation and just return the result.

Notice how we’ve pushed up the conditional relating to benefits logic into the constructor. Now the #calculate method has one less path for conditional execution.

Convert Conditional to Parameter

Once we have the conditional right where we want it (the constructor), we are free to perform Step 7. We’ll inject the new class as a service class:

class PayrollCalculator
  def initialize(
    payroll,
    benefits_calculating_klass: BenefitsCalculator,
    **options
  )
    @payroll = payroll

    @benefits_calculating_klass = benefits_calculating_klass

    @skip_taxes = options[:skip_taxes]
    @skip_donations = options[:skip_donations]
  end

  def calculate
    # (unchanged from previous snippet)
  end
end

When we make this change, we will also need to carefully change all of the callers (Step 8). Any time a caller had used:

PayrollCalculator.new(payroll, skip_benefits: true)

We will need to replace that with:

PayrollCalculator.new(
  payroll,
  benefits_calculating_klass: NullCalculator
)

By doing so we’ve completely removed the conditional logic from the PayrollCalculator class and pushed it up to the caller. If we think back to the start of the post, the caller already knew what it wanted when it specified the control parameter in the first place. We have reduced the total knowledge the PayrollCalculator needs of the outside world and all the different variants it might take.

Final State

From there, we just repeat the steps as we see fit. Fast-forwarding a bit, here’s how the final class might look.

class PayrollCalculator
  def initialize(
    payroll,
    benefits_calculating_klass: BenefitsCalculator,
    tax_calculating_klass: TaxCalculator,
    donation_calculating_klass: DonationCalculator
  )
    @payroll = payroll
    @benefits_calculating_klass = benefits_calculating_klass
    @tax_calculating_klass = tax_calculating_klass
    @donation_calculating_klass = donation_calculating_klass
  end

  def calculate
    result = @benefits_calculating_klass.calculate(@payroll)

    result = @donation_calculating_klass.calculate(
      @payroll,
      result
    )

    @tax_calculating_klass.calculate(
      @payroll,
      result
    )
  end
end

We can restructure this code however we see fit, even going as far as to eliminate the local assignments. Notice how we’ve removed conditional execution from the PayrollCalculator entirely. There’s now only a single path through the execution of the class.

We’ve converted something that was conditional soup into a module that is more easily tested and more easily reasoned about. This was accomplished through a few methodical changes and dependency injection.

The resulting class now only coordinates and collaborates: it does not do any calculation itself. Instead, how it calculates can now be configured by clients. If we add a new calculation method, we just pass that in as a parameter. If we delete a calculation method, that’s easy too.

Conclusion

This refactoring is similar to a few different code smells or best practices such as Tell, Don’t Ask; composition over inheritance; and is a strategy for avoiding cyclomatic complexity within a class. I’ve employed this with a lot of success at work: the resulting code is easier to read, easier to test, and easier to delete.

Keep in mind that this approach will begin layering on the indirection. In the above example, we have a long method signature. We should probably also decide if we need a Parameter Object. A Parameter Object that encapsulates dependencies-to-inject is much more difficult to reason about than a bundle of booleans.

If this blog post piqued your interest, I recommend checking out “Nothing is Something” by Sandi Metz.

Happy refactoring!


Special thanks to Matan Zruya, Jasdev Singh, Sihui Huang, Justin Duke, and Nathaniel Watts for providing feedback on early drafts of this post.