Refactoring: Removing Tangled Control Coupling
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:
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:
- Backfill any tests for what we’re about to change
- Identify all control parameters
- Decide whether to work top-to-bottom or bottom-to-top
- Choose the first/last instance of the control parameter
- Extract the “guts” of the logic being conditionalized into one or more service objects, preferably PFaaOs. Create a class for each conditional branch.
- Create a null service object for the “empty” conditional case
- Inject the new class as a default parameter into the class
- Convert all callers to select which service class they wish to use
- 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.