Destroy All Software Blog

Burn Your Controllers

Posted by Gary Bernhardt on 2011-06-20

In my experience, well-designed Rails controllers tend to do these things:

  1. Delegate to SomeClass in lib.
  2. Choose which SomeClass to delegate to based on higher-level state: is the user logged in? Is he an admin?
  3. Choose where to send the user (to a template or to a redirect) based on what SomeClass did when we delegated to it.

Here's an example of (2): a conditional delegation to SomeClass based on higher-level state:

class PostsController
  def edit
    if current_user.admin?
      @post = Post.find_by_id(params[:id])
    else
      render :nothing => true, :status => 403
    end
  end
end

This action decides whether the user is an admin. If he is, it delegates to SomeClass (the Post model) to find the post and implicitly renders the "edit" view. Otherwise, it responds with HTTP 403 Forbidden.

Here's an example of (3): routing out to different responses based on the result of the delegation:

class SubscriptionsController
  def create
    begin
      SubscribesUsers.subscribe!(params)
    rescue CardAuthorizationFailure
      render :new
    else
      redirect_to catalog_index_path
    end
  end
end

I've used a plain-old Ruby object here instead of a model to illustrate that delegation isn't about models. I've also used an exception to indicate an expected failure. This isn't standard Rails practice, but we'll see why I did it in a moment.

Imagine that all controllers are structured in these ways: they can choose where to delegate, and they can choose where to go after delegating, but all other domain-specific work is left to the delegate itself.

My original point (2), delegating based on state like logged in/out, sounds like routing. Perhaps I should be able to route based on these states, as well as which URL was requested.

My original point (3), choosing where to send the user, also sounds like routing. Perhaps I should be able to declare success/failure renders directly in the routes.

With those two changes, controllers only handle point (1), delegation. Every controller action becomes:

class MyController
  def my_action
    SomeClass.some_method
  end
end

In other words, it doesn't need to exist. The question is, how do we declaratively specify that richer routing such that it's actually more readable than the controllers we write today?

Cramming it into Rails-style routes certainly seems reasonable at first glance:

resource :posts do
  # Admins can update posts.
  put :update => "UpdatesPosts#update!", :require => :admin
  # Other people fall through and can't update posts.
  put :update, :render => :forbidden
end

This is equivalent to the following much-more-verbose Rails code, which uses a standard route directing to a standard controller. The controller makes a decision about the user, then either renders or delegates.

# routes.rb
resource :posts, :only => [:update]

# posts_controller.rb
class PostsController
  before_filter :authenticate_user!

  def update
    if current_user.admin?
      UpdatesPosts.update!(current_user)
    else
      render "forbidden", :status => 403
    end
  end
end

I like the route version a lot better. It condenses that entire controller class into one extra declarative line that says exactly what it means.

But what about routing on the result of the method we delegated to? Here's a possible syntax for error cases: we delegate and, if it raises a particular error, we redirect to another place.

get :show => "ShowsPosts#show",
  NotPublishedError => redirect_to(:index)

Newbies would probably find this even more confusing than standard Rails routing because it's conceptually dense. But I'm not a newbie (if I may be so bold), and I'm tired of writing the same controller code over and over again. I want my interaction with the web framework to be more declarative, freeing me to focus on views and plain-old Ruby objects that implement the application's logic.

I don't know whether this exact scheme is a good idea, but it smells pretty good to me so far. Geoffrey Grosenbach has written about this topic as well, but coming from the other side. His approach would be more compatible with larger controllers; mine is more compatible with heavy delegation. I think that either would be an improvement: the route-controller distinction seems to be losing steam.

(If you're interested in the idea of moving application logic into plain old classes, you might like Destroy All Software's Extracting Domain Objects screencast. )