Refactoring Technique: Replace Conditional With Polymorphism

Published

I came across a blog post recently that suggested replacing case statements with dry-matchers. That’s certainly one way to do it. Let’s take a look at a different approach using a technique that’s been around for more than 20 years.

In Refactoring: Improving the Design of Existing Code, Martin Fowler and Kent Beck introduced the term “code smell”, and gave us recipes for fixing them. They identified common scenarios and gave each code smell a specific name, along with specific named techniques for addressing them.

The code smell we’ll be looking at is “Switch Statements”, also known as “conditionals”. Here’s the example from the blog post I mentioned above:

def shipping_cost(country, weight)
  case country
  when "DE"
    5 + weight * 0.6
  when "GB"
    10
  when "US"
    10
  when "CH"
    raise "We only sell digital products in China!"
  end
end

As an aside, the correct country code for China is CN. CH represents Switzerland.

The problem with this code is that it is switching behavior based on the type of country. Each new country we add requires a new conditional, and the method could end up being hundreds of lines long, making it hard to read and maintain. The longer it gets, the easier it is to make a mistake and introduce bugs. There’s a reason the Ruby Style Guide says methods should be short.

One of the reasons the author of that blog post chose dry-matchers was to be able to catch bugs or failures early. For example, if you try calling the shipping_cost method with a country that hasn’t been defined yet in the case statements, it will return nil. This could lead to problems such as shipping being free for those countries.

If you’re new to Ruby, you can try this yourself in your terminal using IRB, which stands for Interactive Ruby. It’s a quick way to play around with Ruby code.

Launch IRB:

irb

Then copy and paste the shipping_cost method above, and try calling it with France:

shipping_cost("FR", 0)

You should see nil being returned:

irb(main):016:0> shipping_cost("FR", 0)
=> nil

dry-matchers can solve that problem by raising an error if you pass in an undefined country. However, this comes at a cost. You’ve introduced a new dependency in your project, and the code required to make this work is not the easiest to understand at first glance. Furthermore, the shipping_cost method is not significantly improved. The switch statement is arguably still there. You still have to add a match statement for each country, and you can still end up with a huge method. And if you try to implement this pattern on your own without using the dry-matchers gem, like the blog author showed, it still requires adding complex code, along with a bunch of new tests.

A better solution is to use the tried and true technique called “Replace conditional with polymorphism”. Our goal is to end up with a single line in the shipping_cost method that will automatically give us the correct cost for the country we pass in. The first step is to create new classes for each country using the “Replace Type Code with Subclasses” recipe, and then define a cost instance method on each class:

class Shipping
  def initialize(weight)
    @weight = weight
  end

  private

  attr_reader :weight
end

class DE < Shipping
  def cost
    5 + weight * 0.6
  end
end

class GB < Shipping
  def cost
    10
  end
end

class US < Shipping
  def cost
    10
  end
end

class CN < Shipping
  def cost
    raise "We only sell digital products in China!"
  end
end

Now, we need a way to automatically call the correct class based on the country that was passed in. We can do that with some light metaprogramming:

def shipping_cost(country:, weight: 0)
  Object.const_get(country.upcase).new(weight).cost
end

Object.const_get will find the class that matches the country string. For example, if you pass in "DE", it will look for a class called DE. To make this more flexible and resilient, we will call upcase on the country since all our class names are uppercase. That way, it will work whether you pass in "de" or "De" or "DE" or "dE".

To test this out, copy the 2 code blocks above (the various classes and the new shipping_cost method) in your IRB, then try out different countries:

shipping_cost(country: "US")

This will return 10 as expected.

What if we pass in an undefined country?

shipping_cost(country: "Fr")

We get an error, as expected:

NameError (uninitialized constant FR)

What if we create the FR class, but we forget to implement the cost method?

class FR < Shipping
end

We get another error, as expected:

NoMethodError (undefined method `cost' for #<FR:0x00007f90b1ad2520 @weight=0>)

Another improvement I made to the shipping_cost method was to use keyword arguments, as opposed to positional arguments. Before, we were expected to call the shipping_cost method like this:

shipping_cost("CO", 12)

If you were seeing this code for the first time, would you know what CO and 12 represent? Is that Colorado?

By using keyword arguments, we require that they be named when calling the method, so that it’s clear what they represent:

shipping_cost(country: "CO", weight: 12)

Now it’s immediately clear that we’re talking about Colombia, not Colorado, and that 12 represents the weight, although this could be further improved with more specific naming such as weight_in_ounces or weight_in_kilos. I also set a default value of zero for the weight, so that you don’t have to pass it in every time, since it seems like only certain countries make use of it.

One final improvement I’ll mention is for the cost definition for Germany. Using parentheses would eliminate any ambiguity. Right now, it’s not clear whether the intention was to add 5 to the weight, and then multiply the result by 0.6, or multiply the weight by 0.6, and then add 5.

For more on refactoring, I highly recommend these resources: