Rails controllers are a mess.

An ongoing discussion in the Rails community is how, when, and if to test controllers (and how impossible is is to write tests for controllers that are both thorough and not completely isomorphic with implementation).

There are many dimensions to this problem, and it touches a lot of the philosophy of what makes rails rails and what best practices should be: how fat or skinny the controller is, how OO the overall architecture is, where the domain logic lives.

Of all the discussion, not much in the way of practical solutions have emerged. There are some really cool experimental projects (e.g. Focused Controller, Poniard, and another notable one I'm forgetting the name of, starts with a "C") and now a brand new framework that shows much promise, Lotus. These require complete adoption of a new paradigm and don't do much to help the everyday rails project have better code.

There is, however, one set of ideas that has emerged from this discussion which is very easy to implement and immediately offers benefits with essentially no drawbacks:

• Don't use ActiveRecord persistence methods in controllers.
• Don't use ActiveRecord filters.

Instead, write ActiveRecord class methods which do the set of persistence and other related tasks in one logical place. Then, use those methods from your controllers.

Behold:

class Thing
  def self.make_new(creator, params)
    thing = new(params)
    thing.creator_id = creator.id
    thing.save!
    Stats.count("Thing Created", 1)
    thing
  end

  def self.find_and_update(id, params)
    thing = find(id)
    thing.update!(params)
    thing
  end
end

class ThingsController
  def create
    thing = Thing.make_new(current_user,
              params[:thing].permit(:foo, :bar))
    Thing.delay.send_notifications(thing.id)
    @thing = thing.decorate
  end

  def update
    thing = Thing.find_and_update(params[:id],
              params[:thing].permit(:foo, :bar))
    @thing = thing.decorate
  end
end

By itself, this approach has greatly improved my code in the following ways:

• It's easier to to reason about what make_new and find_and_update are for and their role in the system.
• It moves domain logic into the model.
• If I need to make or update a Thing outside of a controller (like in a script or at the command line) I can do that. Before, when DRYing up this controller code or splitting it into methods for better organization, I would create these methods as private classes on the controller -- inaccessible outside of the controller and bad OO design.
• No more filters to mentally chase around -- hooray! If I want to add functionality to what happens in a save or update, I put that directly into these methods.
• I can test Thing.make_new and Thing.find_and_update in a completely functional matter.
• Making changes to what happens during a creation or update is much easier -- I can do it completely inside of the model without thinking about controller behavior changes (and if it does cause controller behavior change, then the change should probably go into the controller).
• I can stub out the behavior of Thing.make_new and Thing.find_and_update in my controller tests, making these tests much simpler.

Controllers and ActiveRecord alike still have many problem (controller actions "input" and "output" are both object state, ActiveRecord is a mix of domain and persistence functionality) — but this pattern can still vastly improve code without having to stray from common Rails conventsions, and the expectations of other code or programmers.