Rails lessons learned: Fat Controllers and Obese Models

So it’s been nearly a year since my rant on rails. In this year I’ve been working on a rather large rails application, trying to help the team tame this 100Ks LOC monolith.

We’ve learned a lot about what wasn’t working for us with the out-of-the-box rails architecture, and how to try to work around that.

In this (and subsequent) posts, I’ll try to describe the issues we’ve faced, and our ways of trying to improve things.
Let’s start with one of the most common issues rails applications face:

Fat Controllers

Too much responsibility in controllers is a very easy pit to fall into in rails application.
I believe that this is because controllers come pre-loaded with so much rails magic and functionality, that it takes very little to get them to be doing ‘too much’.

We all know that controllers should only concern themselves with receiving data from the client and passing data to the views. Like so:

class SkinnyController < ApplicationController

  def create
    result = ...# do something with params[:some_model]
    if result
      @entity_to_show = # get whatever it is we want to show
      @another_param_for_the_view = ...
      render 'some/partial'
    else
      render 'something/else'
    end
  end

end

Things in real life, however, don’t always look like the ‘hello world’ example from RailsGuides.
In real life, we have requirements such as:
1. when an Order is purchased, a payment is taken, the nearest warehouse that has all of the OrderLine items is notified, and an email is sent to the client.

2. the user shouldn’t have the option to pay for the Order if any of the OrderLine items is not currently in stock.

Where do we put all of this logic?

Traditionally, developers would recognize that taking payment and sending emails aren’t domain model responsibilities.
Additionally, signalling to the view which options should be shown, seems to be appropriate for the controller.
So, the first solution most of us would come up with is sticking all of the above logic in the controller.

This is of course a Bad^TM thing.
Firstly, because It puts business logic into the controller, which couples business rules and presentation (Bad!).

Additionally, a point that isn’t immediately evident regarding controllers, is that they’re harder to test than other types of classes.
Therefore, we would like them to have as little logic as possible, because any logic that they contain would be difficult to validate.

The reason why controllers are hard to test is that they don’t have a very well defined API – they just send some messages to a bunch of models, and then render something.

Testing interaction with model classes

Requires us to either:
1. stub them (discouraged for non-statically typed languages, since it’s much harder to make sure the stub’s interface matches the actual class’s interface:
expect(card).to receive(:pay).with(amount, cvc) might pass, even if the pay method actually takes the cvc as a first argument, and amount as the second)
Or
2. read objects from the database and verify them (which poses 2 problems – 1. performance, 2. We’re now testing the model classes, which is not the point of those tests),

This quickly gets very messy the more logic you add on.

Testing that the correct data is rendered

Requires us to either:
1. test the value of the @show_payment_option? instance variable (Problematic – we don’t want to test private attributes)
Or,
2. test the rendered view (Problematic – coupling our controller test with the partial implementation, and introducing more complexity by having to test generated HTML)

For the above reasons – the controllers’ high coupling with the view, and their reduced testability, rails devs are commonly told to aspire to have
“Skinny controllers, fat models”
which is to say –
take as much responsibility away from the controllers, and put it into the models.

Obese models

take as much responsibility away from the controllers, and put it into the models.

As we saw in the previous section, the first part of this sentence is absolutely true; we don’t want a lot of responsibility in our controllers.

However, just blindly sticking all of it into the model classes isn’t great either.

Let’s go back to the 2 examples from before:
1. the system needs to take payment, notify a warehouse and send an email when an Order is purchased,

2. the user shouldn’t have the option to pay for the Order if any of the OrderLine s is not currently in stock.

The first requirement obviously does not belong in a single model class; it concerns many model classes, and possibly some other services.

However, even the second, simpler requirement, is tricky.
At first glance, it makes sense to have a can_pay? method in the Order class.

But, how can the Order know whether a specific item is in stock? Does it need to fetch it from the database? query the Warehouse class? call the external StockManagement service?
Even the simpler case of simply fetching the Item from the database and querying it breaks the isolation of these two classes from each other.
The other described cases are even worse, of course.

Moreover, it’s expected that we would have many more requirements to take some action based on the state of the Order object:
Show the order number in red if there’s an unhandled Complaint attached to it and the order total is more than $100,
stop the order from being shipped if a previous order to the same address was not picked up,
and whatever else your friendly product manager can dream of.

Putting the logic for all of those different things into the model class would create an obese class, littered with has_pending_complaint? , has_previous_denied_shipment? etc., each of them, potentially, concerned with more than just the Order object itself.

So where does business logic, which doesn’t belong in a single Model class, go?

Domain services

This is a well-known concept in other technologies, however, it doesn’t immediately spring to mind when using rails, simply because it’s not a “thing” in rails.

The point of a domain service is to perform an operation which requires interaction with more than one domain object.
The domain service is still in the domain layer, like model classes.
However, it doesn’t hold any data of its own (and it’s not persisted), but rather it is concerned with coordinating several domain classes to represent a process, or a workflow. For example:

class OrderPurchaseService

 def pay

  payment_result = @payment_gateway.pay(@credit_card, @order.total)
  return false unless payment_result

  nearest_warehouse = Warehouse.has_items(@order.order_lines).nearest
  nearest_warehouse.prepare(@order)

  ConfirmationMailer.new(@order).send
  # Whatever else needs to happen here
  return true

 end

end

 

 

(For a more detailed description of domain services and their relation to DDD, see here)

Abstracting a business process into its own class / method helps us solve the issues we discussed previously:

It’s better than having the code in the controller, since now these business rules are removed from the presentation concerns, and have a well-defined output which is more easily testable than a view output.

It’s better than having the code in the model class, since it prevents one model class from being too familiar with other model classes, and stops model classes from implementing methods which aren’t directly related to the model realm (such as has_pending_complaint?)

The controller’s job can now be described as gathering the objects to pass to a domain service (either from the received params, or by reading them from the database), invoking the appropriate domain service, and acting on the result.

The model’s job can now be described as defining the structure and operations over a single domain object.

The Domain Service’s job is to achieve a business goal by using several domain objects / services.

Bonus: Query classes

Query classes are a subset of domain services; these are domain services that don’t cause any changes, and are only used to calculate a result.
Sort of a read-only domain service. (The name is taken, of course, from the Command-Query pattern).
The above-described use cases, such as has_previous_denied_shipment? and has_pending_complaint? can be abstracted into query classes in order to help keep the models from becoming obese.

Conclusion

Rails does not presume to offer all the tools and paradigms required for any application.
It gives a very good starting point for quick and simple applications.

As your application grows, however, the simple 3 concepts of an HTML-view, a controller and a model will struggle to contain all of the more complex logic you’ll need.

In the case of a processes, which require several model classes to cooperate, abstracting it into its own class will help keep your controller and model code singly-responsible and well-isolated.