The “Skinny controller; fat model” mantra so often repeated in the Rails community is only half-right. Most new Rails developers tend to try to stuff too much logic into controllers. That’s definitely true, and it’s definitely an anti-pattern. Skinny controllers are most certainly the way to go. Fat models, however, should also be considered an anti-pattern. This is something you learn pretty quickly when you start working on a Rails app of any appreciable size: models can get unwieldy very quickly.

The real issue is that this problem can’t be solved by shunting the mess down one level; the mess has to be analyzed and refactored. This sounds obvious, but it never fails to astound me how few people realize that refactoring methods away from a model class might be a good idea. I feel that Rails is partially to blame here.
Rails gives you four directories in which to write your code: app/views, app/controllers, app/models, and app/helpers. Lib is there, but it has always felt like a second-class citizen, and, more importantly, has class caching enabled by default, discouraging new developers from stashing useful modules in lib.

So given the choice between views, controllers, helpers, and models, where does your business logic belong? Models, right? Right?

Trick question. None of the above. Models should be viewed strictly as a data integration layer. They interact with your database at a very high level, and do some data manipulation tasks. Ideally, business logic should be stored outside of models in an additional layer.

People that subscribe to this mindset frequently find themselves creating modules in lib (and adding lib to Rails’ autoload path!). This is perfectly valid, and often an excellent choice, but I’d like to propose an alternative, based on a refinement of a pattern I observed Gary Bernhardt using in his fantastic Destroy All Software screencasts. I generally refer to it as the “Behaviour Pattern” when I’m talking about it.

So what’s the Behaviour Pattern?

It’s more or less an adaptation of Martin Fowler’s “Extract Method Object” refactor, taking a little more advantage of ruby’s object model. It simply involves extracting a public method, or a cohesive group of public methods, and all their related private methods from a model, with a specific name. For example:

class Performance
  belongs_to :performer
  # ... lots of associations ...
  # ... lots of validations ...
  # ... lots of short methods ...

  before_save :cache_coordinates

  def cache_coordinates
    self.lat, self.lng = geocode(formatted_address)
  end

  private

  def formatted_address_for_geocoding
    "#{address1} #{city}"
  end

  def geocode(search_term)
    RestClient.get(geocoding_url(search_term)).values_at(:lat, :lng)
  end

  def geocoding_url(search_term)
    "http://example.com/geocode?x=#{CGI::escape search_term}"
  end

end

This is nice, clean, testable code, but when the model grows the the point where it has dozens of methods on it, the private methods inevitably start to get lost, and it becomes difficult to maintain a mental picture of everything the model does. It’s a common occurrence for private methods to stick around in situtations like these long after their only caller has dissapeared. The solution Gary Bernhardt discusses in his screencasts is to create a module named CachesCoordinates, which you write to lib and include into Performance. I take this half a step futher by creating an app/behaviours directory to house these modules:

# app/models/performance.rb
class Performance < ActiveRecord::Base
  include CachesCoordinates
  # ...
end

# app/behaviours/caches_coordinates.rb
module CachesCoordinates
  def self.included(base)
    base.send(:before_save, :cache_coordinates)
  end

  def cache_coordinates
    self.lat, self.lng = geocode(formatted_address)
  end

  private

  def formatted_address_for_geocoding
    "#{address1} #{city}"
  end

  def geocode(search_term)
    RestClient.get(geocoding_url(search_term)).values_at(:lat, :lng)
  end

  def geocoding_url(search_term)
    "http://example.com/geocode?x=#{CGI::escape search_term}"
  end

end

There are a couple minor points that make a big difference to me in terms of later readability:

1. The module’s name literally answers the question “What do you do?”. This module Caches Coordinates.
2. All these modules are always included at the very top of the model.
3. The name of the module usually corresponds to the single public method of that module.
4. The included hook is used to register callbacks.

Point number two creates a great way to skim a model for major functionality. What does Account do? Maybe it:

• ResetsPassword
• CachesAddressCoordinates
• FuzzySearchesAccounts
• CachesCurrentBalance
• VerifiesCreditCard
• and so on.

This creates a great summary of the complicated functionality of a model, and it’s a good first step to more in-depth future refactoring. For example, maybe VerifiesCreditCard info should really be included into a new PaymentMethod class? This gets a bit easier to do, and more importantly, to reason about, when everything is compartmentalized.

The other major benefit of this pattern is that your most important code becomes much more easily testable, as it no longer requires Rails to be loaded, which I’ll blog about next week.

What do you think? Do you have a different way of accomplishing the same thing? Thoughts / Comments / Concerns?