DEV Community

loading...
Cover image for Write clean Object-Oriented code by replacing conditional with polymorphism
Kactus Engineering

Write clean Object-Oriented code by replacing conditional with polymorphism

Alexandre Ruban
Ruby on Rails developer with a strong interest in design (and CSS 😍)
Originally published at kactus.tech ・5 min read

In this article, I will talk about a refactoring called replace conditional with polymorphism that we did recently at Kactus. By the end of the article, you will be able to identify probably many places where you could use this technique in your Rails application. Let's get started.

A little bit of context

Kactus is a marketplace and, because of European regulations, we have a quite complex invoicing system. To keep this article simple, we will only talk about the two different types of invoices that we have:

  • Marketplace invoices are not part of Kactus revenue. They represent money transfers from the client to the provider where Kactus only acts as a middleman.
  • Revenue invoices are issued by Kactus to account for our commission on each booking.

Those two different kinds of invoices are cashed into separate bank accounts. For the accountants to understand which bank account they should send the money, those bank accounts have a different IBAN number. To display the correct IBAN number in the PDF invoices and on the website, we use conditionals:

# Invoice PDF view

<% if @invoice.marketplace? %>
  IBAN number of the marketplace bank account
<% else %>
  IBAN number of the revenue bank account
<% end %>
Enter fullscreen mode Exit fullscreen mode

We also display the IBAN number directly on the website on the show page of each invoice:

# Invoice show page 

<% if @invoice.marketplace? %>
  IBAN number of the marketplace bank account
<% else %>
  IBAN number of the revenue bank account
<% end %>
Enter fullscreen mode Exit fullscreen mode

On the invoice show page, we also display the name of the bank account and a URL for the user to download its associated information:

# Invoice show page 

<% if @invoice.marketplace? %>
  Name of the marketplace bank account
<% else %>
  Name of the revenue bank account
<% end %>
Enter fullscreen mode Exit fullscreen mode

We also use the URL of the bank account information in the mailers to attach the correct PDF to our transactional emails:

# Invoice mailer

if @invoice.marketplace?
  attachments["RIB Kactus.pdf"] = URL of the marketplace bank account
else
  attachments["RIB Kactus.pdf"] = URL of the revenue bank account
end
Enter fullscreen mode Exit fullscreen mode

Can you already see where I am going with this article? Can you see the repeated conditional that's spreading everywhere throughout our application?

# Everywhere

if @invoice.marketplace?
  ...
else
  ...
end
Enter fullscreen mode Exit fullscreen mode

That's what we are going to learn how to solve.

Here comes the new requirement

Our finance director wants us to use two different bank accounts for marketplace invoices:

  • One bank account is for invoices sent to Kactus's premium accounts
  • One other bank account for all other marketplace invoices

What are we going to do? Add more if statements everywhere?

# Everywhere

<% if @invoice.marketplace? && @invoice.recipient.premium_account? %>
  # ...
<% elsif @invoice.marketplace? %>
  # ...
<% else %>
  # ...
<% end %>
Enter fullscreen mode Exit fullscreen mode

Of course not, that would be a maintenance nightmare. It's time for a refactoring.

The replace conditional with polymorphism refactoring

Creating the missing bank account object

When I was describing the problem, there is something interesting to notice. I talked about bank account but when writing code, I never wrote bank account anywhere. We are missing an important object here.

# app/models/bank_account.rb

class BankAccount
end
Enter fullscreen mode Exit fullscreen mode

That's already a huge win. We have a new object in our application that represents an important concept in real life. Note that this is a Plain Old Ruby Object, it does not inherit from ActiveRecord::Base or ApplicationRecord. You can of course add simple models like this one into your app/models folder.

Removing the if statements with a factory method

A factory method is a method that's sole purpose is to create other objects. This sounds complicated. Let's explain it easier: a factory method is the only place where if statements related to choosing bank accounts reside. By convention, we name it ".for" and it's a class method.

# app/models/bank_account.rb

class BankAccount
  # The factory `.for` method is the only place in the application
  # where the if statements should appear
  def self.for(invoice)
    if invoice.marketplace? && invoice.recipient.premium_account?
      PremiumMarketplace
    elsif @invoice.marketplace?
      DefaultMarketplace
    else
      Revenue
    end.new
  end
end
Enter fullscreen mode Exit fullscreen mode

Notice how our factory method returns a new instance of BankAccount::PremiumMarketplace, BankAccount::DefaultMarketplace or, BankAccount::Revenue classes? That's exactly what a factory method is used for. Let's create those 3 missing objects now:

Creating the missing objects

All the new objects here should have the same interface. If they have the same interface, they can be substituted. That's what polymorphism is. Let's write the code of our 3 new objects:

# app/models/bank_account/premium_marketplace.rb

class BankAccount::PremiumMarketplace
  def name
    "Premium marketplace bank account name"
  end

  def iban_number
    "Premium marketplace bank account IBAN number"
  end

  def iban_url
    "Premium marketplace bank account IBAN URL"
  end
end
Enter fullscreen mode Exit fullscreen mode
# app/models/bank_account/default_marketplace.rb

class BankAccount::DefaultMarketplace
  def name
    "Default marketplace bank account name"
  end

  def iban_number
    "Default marketplace bank account IBAN number"
  end

  def iban_url
    "Default marketplace bank account IBAN URL"
  end
end
Enter fullscreen mode Exit fullscreen mode
# app/models/bank_account/revenue.rb

class BankAccount::Revenue
  def name
    "Revenue bank account name"
  end

  def iban_number
    "Revenue bank account IBAN number"
  end

  def iban_url
    "Revenue bank account IBAN URL"
  end
end
Enter fullscreen mode Exit fullscreen mode

See how simple those objects are? Every Ruby developer can understand them very easily. The last step is now to clean our views, mailers, and models that depend on the conditional seen earlier. Let's do it.

Cleaning up our previous code

First, let's make it really easy for us to access the bank account. On the Invoice model, we will create a getter to access the correct bank account for a given invoice.

# app/models/invoice.rb

class Invoice < ApplicationRecord
  def bank_account
    @bank_account ||= BankAccount.for(self)
  end
end
Enter fullscreen mode Exit fullscreen mode

And now let's see the benefits of our refactoring everywhere in our application where we used to have if statements.

# Everywhere

# No more if statemements, we replaced conditional
# with polymorphism

@invoice.bank_account.name
@invoice.bank_account.url
@invoice.bank_account.iban_number
Enter fullscreen mode Exit fullscreen mode

There are other advantages, to this architecture:

  • Next time, when our finance director asks us to add a new bank account, it will be really easy to do so. Simply add a conditional at a single place in the factory method and add a new class for the additional bank account.
  • When a new member of the team is given the task to change something related to bank accounts, the architecture is already in place. No one will need to struggle with maintaining the same conditional everywhere throughout the application.

Conclusion

The replace conditional with polymorphism is a well know refactoring that will provide value immediately in your team by making maintenance easier and prevent bugs in the future. If you want to learn more about it, you can read the amazing 99 bottles of OOP book by Sandi Metz.

Discussion (2)

Collapse
epigene profile image
Augusts Bautra

Great stuff!
I am not a fan of putting non-AR classes in app/models though. I believe there is value in only having AR classes there, by convention. Perhaps app/value_objects is a better place for extracts like that.

Collapse
alexandreruban profile image
Alexandre Ruban Author

Hey! Thank you very much for your comment!

We could move non-ActiveRecord models in app/value_objects if we wanted to. However, when looking at a Ruby object, we feel more concerned about what it does - the methods it responds to - rather than its class/inheritance hierarchy.

When you look at the Rails source code, you can see that some non-ActiveRecord objects like ActiveStorage::Variant are in app/models. DHH also says that all models are not necessarily ActiveRecord in his great video series "On writing software well"; I highly recommend it if you want to see a bit of Basecamp's code!