Maintainable Spree: Command pattern

Piotr Matlag

April 22, 2020

Night mode

For some time now my work at Upside has been connected to an online store development for one of our clients. The framework we use for this project is Spree. It offers programmers a rich set of built-in models and features that would easily suffice on their own to build a simple online store for a small company. When it's not enough, Spree's expandable architecture allows programmers to bake in new behaviors that suit their use cases. The official way to do this is via monkey patching existing classes with your custom features. Example from Spree developer documentation:

module MyStore
module Spree
module ProductDecorator
def some_method
...
end
end
end
end

::Spree::Product.prepend MyStore::Spree::ProductDecorator if ::Spree::Product.included_modules.exclude?(MyStore::Spree::ProductDecorator)

This mechanism is universal and works the same way for models, as well as for controllers.

Over the course of project's growth our team faced some major questions:

Where to put non-trivial application logic?
How to share that logic between components?
How to do all of that and also keep the size of our classes under control?

Rails ecosystem, despite being famously opinionated in some aspects, does not give a clear answer.

Spree's monkey-patching mechanism hints at how we can weave our logic into its components, but it's not very helpful with determining where to put it exactly. Furthermore, Spree classes are pretty big on their own already - a file containing Spree::Product implementation in Spree 4.1 has 455 lines in total. Extending them with custom logic will produce even bigger files, that are even harder to understand and navigate.

We tried many different ways of addressing this reusable logic problem and, in the end, one of them emerged as an optimal mix of convenience, separation of responsibilities and general tidiness - the command design pattern. It allows us to keep even complicated logic as a system of interconnected, manageable classes with well defined responsibilities and interfaces. Let me use an example to demonstrate commands' usefulness. For the sake of simplicity I'll leave out the details of Spree code while also trying to stay close to its principles with some mock classes written in plain Ruby on Rails.

You're an e-commerce store owner, Harry

Imagine you have an online store. You conduct business in many countries and need to support multiple currencies. A thin slice of your application might look like this:

AppConfig[:currencies] = ['EUR', 'PLN', 'SEK']

class ProductsController
def create
# Logic for creating product
end

def update
# Logic for updating product
end

# More code ...
end

class Product
has_many :prices
has_one :default_price,
-> { where currency: 'EUR' },
class_name: 'Price'

# ...
end

class Price
belongs_to :product

validates :amount, numericality: {
greater_than_or_equal_to: 0,
}

validates :currency, inclusion: {
in: AppConfig[:currencies]
}

# ...
end

A typical use case would be something along these lines:

Administrator adds a new product supplying its price in EUR
Prices for every other supported currency (in this example: PLN, SEK) are calculated based on the EUR price and associated with this product

In a similar fashion we would like to update all of the product's prices when the store administrator updates it's EUR price. How can we get this done with Rails?

Skinny model, fat controller

The first instinct might be to put business logic right where we need it - in the controller:

class ProductsController
after_action :update_prices, only: %i[create update]

def create
# Logic for creating product
end

def update
# Logic for updating product
end

private

def update_prices
AppConfig[:currencies].each do |currency|
# Make sure price for a given currency exists
# and is up to date
end
end
end

We define update_prices as an after filter, which causes it to run after every create or update action. Inside we make sure that, for every currency our shop supports, there is a price associated with the current product. Keep in mind, that logic responsible for this might be bulky - we need to iterate over all of the product's prices, ensure that our conversion rates are up to date (which will probably involve making some requests to an external API) and finally - calculate and assign a price in currency.

This approach represents a famous "Skinny model, fat controller" anti-pattern. It causes a bunch of problems:

It breaks the Single Responsibility Principle - controller is not only responsible for serving incoming requests, now it needs to know how to update products' prices
Controllers quickly get large, bulky and therefore - hard to understand.
Testing controllers is harder than testing other classes.
Price update logic is confined to just one controller. If a need arose to update product prices in some other part of our store, we would need to replicate the update logic there. We would then have not one, but two (or more...) places to keep in mind if that logic ever changes and needs to be updated.

It's very easy to spot a problem with fat controllers when we introduce a new mechanism to our application - a worker that goes off every night, updates conversion rates for supported currencies and then updates all prices to reflect new rates. Right now there is no way for that worker to reuse the price update logic from the controller, so we would need to copy it and end up with duplicate code.

Fat model, skinny controller

One way to overcome problems of the fat controller is to put business logic inside corresponding models:

class ProductsController
after_action :update_prices, only: %i[create update]

# ...

def update_prices
@product.update_prices
end
end

class Product
has_many :prices
has_one :default_price,
-> { where currency: 'EUR' },
class_name: 'Price'

def update_prices
AppConfig[:currencies].each do |currency|
# Make sure price for a given currency exists
# and is up to date
end
end

# ...
end

Instead of just performing price updates in the controller, now we delegate that responsibility to the product. This immediately makes many of our previous problems go away:

Controller no longer needs to know the details of product implementation. It just tells the product to update its prices.
Controllers become much leaner, which makes them much easier to reason about.
Testing models is much easier than testing controllers
Price update can now be ordered across many different parts of an application

While this approach is surely better than putting logic in the controller, there is one problem that we didn't solve yet.

Earlier we were putting our controllers at risk of getting bulky and hard to understand. Now, the same thing happens to our models. If we just put all of application logic inside domain models they will quickly turn into monolithic blobs that nobody understands.

Just take a look at what happens to our Product when we implement a little more of a price update logic:

class Product
has_many :prices
has_one :default_price,
-> { where currency: 'EUR' },
class_name: 'Price'

def update_prices
AppConfig[:currencies].each do |currency|
conversion_rate = ConversionRate.find_by(
from: default_price.currency,
to: currency
)

price = price_in(currency)
recalculate_price(price, conversion_rate)
end
end

def price_in(currency)
prices.find_or_create_by!(currency: currency)
end

def recalculate_price(price, conversion_rate)
new_amount = price.amount * conversion_rate.multiplier
price.update!(amount: new_amount)
end

# ...
end

Not so bad yet, but that's just the tip of an iceberg. We will need a lot more code to implement a fully functional product model with all expected features. How can we keep the complexity of our models at bay?

Isolating logic into the command class

The way I handle it is by taking away logic from the model itself and putting it into external command classes. I pretty much follow these steps:

Identify behaviors and algorithms connected connected to a model. (updating product prices is a good example)
Split those behaviors into basic, self-confined subroutines. This way, even when we deal with a complex operation, we can reason about its parts independently. What we're usually left with at this point is a dependency tree with the main operation as a root and helper subroutines as branches.
Define the data flowing between isolated execution units, therefore defining their interfaces
Put the result in separate command classes with interfaces defined in the previous step. Use subroutines classes as dependencies of the main class

Let me give you an example:

class ProductsController
after_action :update_prices, only: %i[create update]

# ...

def update_prices
UpdatePrices.new.call(product: @product)
end
end

class UpdatePrices
def initialize(currencies: AppConfig[:currencies])
@currencies = currencies
end

def call(product)
@currencies.each do |currency|
conversion_rate = ConversionRate.find_by(
from: product.default_price.currency,
to: currency
)

price = product.price_in(currency)
recalculate_price(price, conversion_rate)
end
end

def recalculate_price(price, conversion_rate)
new_amount = price.amount * conversion_rate.multiplier
price.update!(amount: new_amount)
end
end

class Product
has_many :prices

# ...
end

Splitting application logic into commands allows us to have one, authoritative place to perform operations on models. Should this logic ever change, we just need to take care of this one place.

Command architecture is very modular. Suppose that at some point in the future the logic behind recalculating prices grows significantly. We can keep UpdatePrices clean by simply isolating price recalculation into its own command class:

class UpdatePrices
def initialize(
currencies: AppConfig[:currencies],
recalculate_price: RecalculatePrice.new
)
@currencies = currencies
@recalculate_price = recalculate_price
end

def call(product)
@currencies.each do |currency|
conversion_rate = ConversionRate.find_by(
from: product.default_price.currency,
to: currency
)

price = product.price_in(currency)
@recalculate_price.call(price, conversion_rate)
end
end
end

class RecalculatePrice
def call(price, conversion_rate)
# Some heavy computations
end
end

That way, despite the logic of our application growing, we can still keep the code in manageable chunks with well defined interfaces.

Remember the worker responsible for updating product prices every night? What happens if we want to change the API that it uses to fetch them? If we use commands, switching to a new service is as easy as implementing a command and injecting it as a dependency:

module OldAPI
class FetchConversionRates
def call(currencies)
# ...
end
end
end

module NewAPI
class FetchConversionRates
def call(currencies)
# ...
end
end
end

class UpdateConversionRates
def initialize(
fetch_conversion_rates: OldAPI::FetchConversionRates.new,
currencies: AppConfig[:currencies]
)
@fetch_conversion_rates = fetch_conversion_rates
@currencies = currencies
end

def call
conversion_rates = @fetch_conversion_rates.call(@currencies)

# ...
end
end

class UpdatePricesWorker
def perform
# ...

# update_conversion_rates = UpdateConversionRates.new
update_conversion_rates = UpdateConversionRates.new(
fetch_conversion_rates: NewAPI::FetchConversionRates.new
)
update_prices = UpdatePrices.new

update_conversion_rates.call
Product.all.each do |product|
update_prices.call(product)
end

# ...
end
end

One more benefit of keeping algorithms as separate classes - testing them is very effective. They do not have many dependencies except, perhaps, other commands that we can test separately down to the most basic ones. The benefit of testing commands is big, considering that's basically the place for our non-trivial application logic. They perfectly fall into the top left quadrant of Steve Sanderson's unit tests benefit vs cost graph.

A word of caution

As with everything, command pattern is not a silver bullet for all software engineering problems. In particular, the process of isolating application logic into commands should be really well thought-through.

If we just put everything into one God Command with a million responsibilities, it would be no better than just putting it directly into a model. We'd still end up with a complex, hard to understand mess.

If we go all the way in the opposite direction and create a multitude of commands depending on each other to do very minute things, we will likely end up passing a whole lot of data between them. That way we just disperse the complexity across the tightly-coupled network of miniature commands, which makes it even harder to understand.

Instead, we should focus on splitting our logic into chunks with very clear responsibilities. It's a classic Single Responsibility Principle. A command works best if it does exactly one, and only one thing.

Summary

The command design pattern might be a very useful tool for keeping non-trivial application logic isolated from the rest of the application in a manner, that is easy to reason about and test. Because of it's modularity it is expandable and allows the logic to grow, while keeping complexity under control.

When splitting desired features into commands we should be especially careful to keep them confined into exactly one responsibility. That way we end up with a system, that has exactly one, authoritative place for the implementation of every behavior. Time put into thinking it through will pay off over time, when we need to expand our codebase.

‍

Back to blog