GIANT ROBOTS SMASHING INTO OTHER GIANT ROBOTS

Written by thoughtbot

Nosy models

Ruby’s “mixins” provide a simple, middle-ground option to developers that wish to include reusable functionality in several classes. When using C++-style multiple inheritence, class hierarchies quickly become muddy, and it becomes necessary to look in several places to find the “core” behavior of a class. Mixins allow you to move reusable methods into modules. These methods can provide an extended interface based on the base functionality of your class (such as the Enumerable module), or they can be used to include “helper” methods (such as ActiveSupport’s Memoizable). Using mixins to include reusable behavior, such as helper methods or interface extensions, can greatly reduce repetition and bloat in your code.

Ruby stew

However, mixins can easily be abused. One common use of Ruby modules is to “break up” a large class that is becoming unmanageable. Let’s say you have a simple Article model:

class ArticleTest < ActiveSupport::TestCase
  should_validate_presence_of :title, :abstract, :body, :author
end

class Article < ActiveRecord::Base
  validates_presence_of :title, :abstract, :body, :author
end

So far so good. Now let’s say you decide you want to submit articles to an external service upon publication, such as the crossref service for DOIs:

class ArticleTest < ActiveSupport::TestCase
  should_validate_presence_of :title, :abstract, :body, :author

  should "submit to crossref after being saved" do
    title = 'a title'
    # other constants
    # expectations for net/http
    Factory(:article, :title => title, ...)
  end
end

class Article < ActiveRecord::Base
  after_create :submit_to_crossref

  private

  def submit_to_crossref
    # convert article to xml
    # format a crossref request
    # connect to crossref using net/http
  end
end

Very quickly, this submit_to_crossref method will get unwieldy, and a good programmer will refactor it into several small methods. Now you’ve added maybe a hundred lines to your simple article model, and the tests are probably more complicated. Once you start handling possible failure cases (such as unexpected server responses or connection errors), there’s probably more crossref code than article code.

At this point, the most obvious symptom of your problem is the fact that you need to look through all of this crossref code when dealing with an article, and you probably rerun all the crossref tests when changing your article model. One quick way to deal with this symptom is to quickly move all the methods from your model into a module and include it:

class Article < ActiveRecord::Base
  after_create :submit_to_crossref
  include Crossref
end

module Crossref
  private

  def submit_to_crossref
    # call helpers to format and submit the request
  end

  # helper methods related to crossref
end

At that point, you can still test this module through article, but your article tests are still testing too much functionality, and you’d be in worse trouble if you decided to add Crossref support to another model. For now, though, let’s just assume that you only want to submit Article records to Crossref. You’ve still only treated a symptom of the core problem: dealing with crossref is really none of your article model’s business. It shouldn’t be tested with your article code, and it shouldn’t be part of the model’s interface. Moving the code into a module spreads the behavior out, so that you need to look in several places to find out what’s going on in Article. However, the concerns are still completely mixed, and testing the Crossref behavior without having an instance of Article is impossible, which leads to obscure and fragile tests.

Adding a class

There’s a basic appeal to the “one domain concept, one model, one class” approach you find in a lot of Rails applications, but that approach breaks down in all but the most basic applications. Your models will likely have to deal with concepts that don’t directly concern them (and aren’t part of the core domain concept), and you’ve seen that mixins don’t seem to help with these bloated classes. So, where else can you put this separate behavior? A new class:

class Article < ActiveRecord::Base
  after_create :submit_to_crossref

  private

  def submit_to_crossref
    CrossrefRequest.new(:title => title,
                        # other info for the request
                        :author => author).submit
  end
end

class CrossrefRequest
  def initialize(attributes)
    # set attributes
  end

  def submit
    # call helpers to format and submit the request
  end

  private

  # helper methods related to formatting/submission
end

Now you have a discrete class that deals only with crossref requests. None of its helpers end up in your Article model, and you can create focused tests just for crossref. You no longer need to guess where the “submit_to_crossref” method comes from – we replaced the call to a mixed in method by instantiating another class, and it’s much easier to locate a constant. If you’re a fan of mocking, you can also easily test that Article uses CrossrefRequest appropriately, so you’ll get immediate feedback if your callback chain gets out of whack:

class ArticleTest < ActiveSupport::TestCase
  should_validate_presence_of :title, :abstract, :body, :author

  should "submit to crossref after being saved" do
    title = 'a title'
    request = mock('crossref-request', :submit => true)
    CrossrefRequest.
      expects(:new).
      with(:title => title, ...).
      returns(request)
    Factory(:article, :title => title, ...)
  end
end

Whether you decide to mock out CrossrefRequest or take a state-based testing approach, I recommend also having acceptance tests in place to ensure the components collectively result in the feature you’re trying to provide. This class will also be much easier to reuse, and the temptation to couple the two models will be lower, as methods in your model are not directly available in CrossrefRequest – everything necessary from the request must be passed in to the constructor.

Ruby smoothies

It’s much easier to test and maintain small, discrete objects, so don’t be afraid to pull functionality out of your models and into classes specific to their concerns. Next time you end up with a model that hurts your editor, take a look at the method list – I bet a lot of that behavior is none of your model’s business!