I usually think of Jamis Buck's “Skinny Controller, Fat Model” as “Smart Model, Dumb Controller.”

After accepting the notion that models should be fat and controllers should be skinny, the important question is how do you know when a controller is too fat? Having too many lines of code is certainly a smell, but it doesn't definitively indicate if a controller is bigger than it should be.

So why dumb controller instead of skinny controller? Controllers should be skinny because they have few responsibilities. When a controller is fat, it's likely because it knows too much - it's too smart. In most cases, it means logic that belongs in the model ended up in the controller.

In general, a skinny controller is a dumb controller. The model contains the business logic, and the controller is just responsible for making a couple method calls to the model and deciding which template to render or page to redirect to.

I found a good example to demonstrate my point from the open source blog engine Mephisto. Here's an example of a two line controller action that is a little smarter than it should be.

def index
  @enabled, @disabled = @users.partition { |u| u.deleted_at.nil? }
  @users = @enabled + @disabled
end

This controller knows that a user with a nil deleted_at value is enabled. The controller shouldn't be that smart - the logic that determines if a user is enabled or disabled based on some state should be encapsulated in the User class. This controller action should look like this:

def index
  @enabled, @disabled = @users.partition { |u| u.enabled? }
  @users = @enabled + @disabled
end

This action wasn't too fat, it only contains two lines - it was just too smart.

Looking at Jamis's example, here is his overweight controller.

def index
  @people = Person.find(
    :conditions => ["added_at > ? and deleted = ?", Time.now.utc, false],
    :order => "last_name, first_name")
  @people = @people.reject { |p| p.address.nil? }
end

And after a diet, here's his skinny controller.

class PeopleController < ActionController::Base
  def index
    @people = Person.find_recent
  end
end

Moving the logic to the model sure does make the controller look nice, but what are the benefits other than aesthetics?

The greatest benefit is again, encapsulation. If the controller is querying for people where added_at > Time.utc.now and deleted = false, then any change to the internal state of a Person means the callers need to be updated. Idealistically, only methods in Person would need to be updated. In a small application, it may not make a big difference. But as the application grows, as more and more areas of the application need to display people, it will become more painful to make changes if you don't have good encapsulation.

Posted on 2008-06-24 | permalink | del.icio.us