Refactoring with Hexagonal Rails

I recently watched the Hexagonal Rails talk that Matt Wynne gave at GORUCO 2012. In the video, Matt points out that Rails makes you very productive at the start of a project, but that they often become harder to work with as they mature. Perhaps we can find inspiration for a solution in Alistair Cockburn's hexagonal architecture?

I've been experimenting with Matt's ideas on Agile Planner, and wanted to share how I'm currently writing the more complicated controller actions.

As a Rails developer you don't get many chances to sharpen your OO design skills; we spend most of our time dropping snippets of code into the structure that Rails provides for us. Before I picked up Rails I was working on a large Python system, and was lucky enough to be introduced to a book called "Object Design, Roles, Responsibilities and Collaborations" by Rebecca Wirfs-Brock. The book makes a strong case for reducing the number of responsibilities of each object in a software system. I found it to be a great way to think about how to design my objects.

What are the responsibilities of a Rails controller? Two spring immediately to mind:

1. To serve as the application's interface to the HTTP protocol.
2. To trigger an action on a domain object (e.g. a model) in response to messages received via HTTP.

Controllers often take on a third responsibility: Querying model objects to find out whether they were able to carry out their tasks successfully, and initiate further actions on domain objects in response to the outcome.

What do I mean by "querying"? Here's an example:

def update
  if @card.update_attributes(params[:card])
    redirect_to card_path(@card), notice: 'Card saved'
  else
    flash.now[:alert] = 'Ooops, something went wrong'
    render action: 'edit'
  end
end

The controller has sent a message to the card object, asking itself to update its values. The controller has then asked the card whether or not the update was successful, by querying the return value in the if statement.

There's nothing wrong with that; at this level of complexity the standard Rails approach let's us add features very quickly. Let's look at what happens when we need to take more actions on our domain objects, in addition to updating the card.

The update action needs to cope with more than just updating the text on the card. It's also responsible for:

• Assigning the card to a new iteration.
• Moving the card to another column on the Kanban wall.
• Re-prioritising it within the backlog.

None of the above can be saved by the update_attributes call, so the update action needs to get a bit smarter.

If we were to write it all out in the update action it'd look like this:

def update
  if params[:iteration]
    @card.iteration = @card.project.find_iteration(params[:iteration])
  end
  if params[:column]
    @card.column = @card.iteration.columns.find_by_name(params[:column])
  elsif @card.column.blank?
    @card.column = @card.iteration.columns.first
  end
  if params[:priority]
    Prioritiser.reprioritise_card(@card, params[:priority])
  end
  if @card.update_attributes(params[:card])
    redirect_to card_path(@card), notice: 'Card saved'
  else
    flash.now[:alert] = 'Ooops, something went wrong'
    render action: 'edit'
  end
end

Ouch - that's horrible! My initial refactoring read slightly better, and looked something like this:

def update
  assign_iteration
  assign_column
  update_card_priority
  if @card.update_attributes(params[:card])
    redirect_to card_path(@card), notice: 'Card saved'
  else
    flash.now[:alert] = 'Ooops, something went wrong'
    render action: 'edit'
  end
end

private
  def assign_iteration
    if params[:iteration]
      @card.iteration = @card.project.find_iteration(params[:iteration])
    end
  end

  def assign_column
    if params[:column]
      @card.column = @card.iteration.columns.find_by_name(params[:column])
    elsif @card.column.blank?
      @card.column = @card.iteration.columns.first
    end
  end

  def update_card_priority
    if params[:priority]
      Prioritiser.reprioritise_card(@card, params[:priority])
    end
  end

That's not so bad, but there are a couple of code smells here.

1. The new private methods are only usable from the update method, but they're accessible from the entire controller. Whenever I'm working on any of the other actions in this controller those two methods will be cluttering up my editor.
2. The controller is sending four messages to assign new data and several to query existing values. That isn't the controller's responsibility; why can't it just send a single message to a domain object to tell it to update the card?
3. There's knowledge about how the data is stored encapsulated within the order that these messages are sent to the model. While it's embedded in a controller action, that knowledge isn't re-usable.

Is it worth doing anything to deal with these problems? As Matt says in his talk, having your controller talking directly to your model enables you to move rapidly during the early days of a project.

Maybe it's an appropriate approach for your application right now, but if you want to tidy it up, what can you do?

Before reading on, watch the talk video. It'll set the scene by explaining the reasoning behind a hexagonal architecture and will put the code you're about to read in context.

A hexagonal refactoring

The first thing we need to do is to push all this knowledge of how to update a card into a new object. I created a PlanChange class and moved the code for interacting with ActiveRecord into it.

My first PlanChange class looked like this:

class PlanChange
  def perform(card, iteration_name, priority, column_name, attributes)
    assign_iteration(card, iteration_name)
    assign_column(card, column_name)
    Prioritiser.prioritise_card(card, priority)
    card.update_attributes(attributes)
  end

  def assign_iteration(card, iteration_name)
    if iteration.present?
      card.iteration = card.project.find_iteration(iteration_name)
    end
  end

  def assign_column(card, column_name)
    if column_name
      card.column = card.iteration.columns.find_by_name(column_name)
    elsif card.column.blank?
      card.column = card.iteration.columns.first
    end
  end
end

I moved most of the tests for the update action into a new test class for the PlanChange. Everything felt very tidy.

My controller had shrunk down a bit too:

class CardsController
  def update
    iteration = params[:iteration]
    priority = params[:priority]
    column = params[:column]
    change = PlanChange.new
    change.perform(card, iteration, priority, column, params[:card])
  end
end

The only thing we're missing is any way to respond to the request in the controller; we've lost the redirect_to and render calls.

In his video Matt shows code that passes the controller object into his domain object, so that the domain object could run a success/failure callback method on the controller.

class PlanChange < Struct.new(:listener)
  def perform(card, iteration, priority, column_name, attributes)
    assign_iteration(card, iteration)
    assign_column(card, column_name)
    Prioritiser.prioritise_card(card, priority)
    if card.update_attributes(attributes)
      listener.card_update_succeeded(card)
    else
      listener.card_update_failed(card)
    end
  end

  # ... other methods removed for clarity ...
end

The PlanChange object would then be instantiated in the controller like this:

change = PlanChange.new(self)

…and the callbacks are nice and simple:

class CardsController

  # ... other methods removed for clarity ...

  def card_update_succeeded(card)
    redirect_to card_path(card), notice: 'Card saved'
  end

  def card_update_failed(card)
    flash.now[:alert] = 'Ooops, something went wrong'
    render action: 'edit'
  end
end

It works nicely. Rails gives public controller methods special treatment, and I wondered if I could avoid putting them on the controller. When I first saw Matt's callbacks I immediately thought of the observer pattern (also known as "Publish-Subscribe"). I've used it a lot in the past when working on large Python projects, and always appreciated the effect it had on my code. I decided to give it a try.

Introducing the Publish-Subscribe pattern

The pattern itself is very simple; a publisher object that emits events provides API for other objects (subscribers) to register their interest. Whenever the event occurs, the publisher sends a message to all the subscribers. I created a lib/publisher.rb file and added this code to it:

module Publisher
  def add_subscriber(object)
    @subscribers ||= []
    @subscribers << object
  end

  def publish(message, *args)
    return if @subscribers.blank?
    @subscribers.each do |subscriber|
      subscriber.send(message, *args) if subscriber.respond_to?(message)
    end
  end
end

By including the Publisher module in PlanChange I gave it the ability to broadcast messages to interested parties.

Now I just needed an object to subscribe to the events. I created an inner class inside CardsController to handle re-directing/rendering. I liked it, so dropped it in a gist and sent it to Matt. Matt copied it to Steve Tooke who made a delightful improvement using Ruby's SimpleDelegator class. I ended up with this:

class CardsController
  class PersistenceResponse < SimpleDelegator
    def card_updated(card)
      redirect_to card_path(card), notice: 'Card saved'
    end

    def card_update_failed(card)
      render action: 'new', alert: 'Ooops!', locals: { card: card }
    end
  end

  def update
    # ... set local variables from params hash ...
    change = PlanChange.new
    change.add_subscriber(PersistenceResponse.new(self))
    change.perform(card, iteration, priority, column, params[:card])
  end
end

Things to note:

• PersistenceResponse can call controller methods as it delegates all its calls to the controller object that was passed into it when it was created.
• The PersistenceResponse can't create any controller instance variables for use in the view, but that's no bad thing.

The final version of PlanChange now publishes an event to all its subscribers after it has tried to save to the database:

class PlanChange < Struct.new(:card)
  def perform(iteration, priority, column_name, attributes)
    assign_iteration(card, iteration)
    assign_column(card, column_name)
    Prioritiser.prioritise_card(card, priority)
    if card.update_attributes(attributes)
      publish(:card_updated, card)
    else
      publish(:card_update_failed, card)
    end
  end

  # ... other methods removed for clarity ...
end

I also modified it to take a card in its initializer, rather than having it passed in with every call to the perform method.

You may be thinking that the observer pattern, with the addition of lib/publisher.rb is overkill. There's nothing wrong with the listener based approach that Matt used in his video, but I rather like the language of publishing a message. It's a great way to think about what's going on.

Since I wrote the code in this post, I've started logging key events that track how my users are using the app in KISSmetrics. I created a Metrics class that can respond to a long list of messages (e.g. "card prioritised", "iteration closed") and then submit data to KISSmetrics in a background thread. I added Metrics as a subscriber to all my updater/creator objects in the relevant controller actions. Plugging it in felt very easy.

I couldn't help but smile when I realised that I'd be able to test PlanChange without executing any of the new metrics logging code. If all this work was done directly in the controller it would all get run together.

With this approach the unit tests for PlanChange will consequently run noticeably faster. Breaking your app apart into objects with fewer responsibilities will do that for you.

When should you use it?

As usual with architectural considerations, I'd recommend this refactoring is applied with care. Its suitability depends on the context in which you find yourself. If overused it could complicate and obfuscate your code rather than making it easier to understand.

While I really like how this approach has impacted my code's design, this write up isn't intended to serve as hard and fast advice. I'm just sharing where my experiments have led me.