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 %>
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 %>
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 %>
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
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
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 %>
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
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
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
# 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
# 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
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
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
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.
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.
Top comments (3)
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.
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!
PS if you ever get tired of maintaining your invoicing features take a look at spaceinvoices.com, our API makes implementing invoicing for marketplaces a breeze.