Rails tip #72: hands off other’s private parts

In Ruby on Rails the most common way to pass data from the controller to the views is by allowing views direct access to the controller’s instance variables. Encapsulation is one of the cornerstones of software engineering. Why is it thrown out the window for views? Allowing external code access to an objects private parts is just wrong! Seriously, god kills a kitten every time someone does this. What is worse this anti-pattern is perpetrated in pretty much every tutorial on Rails Guides and every other RoR tutorial i have ever seen.

Forgoing encapsulation for controllers has the same issues as it has for any other type of object. You couple the implementations of the two components in a way that has very high connascence. This means that if either the view or the controller change it is very likely to require a change to the other. All of this adds up to more fragility and maintenance and less fun.

Consider this basic posts controller implemented in the current nominal RoR style


class PostsController < ApplicationController
  def index
    @posts = Post.all
  end
  
  def new
    @post = Post.new
  end

  def show
    @post = Post.find params[:id]
  end
    
  def update
    @post = Post.find params[:id]
    @post.update_attributes params[:post]
  end
  
  def destroy
    @post = Post.find params[:id]
    @post.destroy
  end
end

That repetition around finding the post sets my teeth on edge. You could fix the repetition by pulling it out into a before filter but that complexifies the code and makes the already deep stack even deeper. The indirect invocation nature of filters makes it easy to overlook their existence and it requires you remember which actions they get invoked on and which ones they don’t. You even have to keep that stuff in mind when writing partials that are many levels of inclusion removed from the controller. Having to keep all that state in your brain slows you down. It also increases the risk of misremembering something and writing a view that doesn’t work.

Fortunately, Rails has a good solution to this problem. Consider the following refactor


class PostsController < ApplicationController
  def update
    post.update_attributes params[:post]
  end
  
  def destroy
    post.destroy
  end
  
  def post
    @post ||= if params[:id]
                Post.find params[:id]
              else
                Post.new
              end
  end
  helper_method :post
  
  def posts
    @posts ||= Post.all
  end
  helper_method :posts
end

And an accompanying view partial


<h1><%= post.title %></h1>
...

Now we have two efficiently memoized accessor methods for the data we want to expose to the views. The resulting code is better in several ways

  • The code is DRYer. If we want to implement visibility control for posts we can do it in exactly one place.
  • The logic of the action is easier to follow because the data lookup code is called explicitly.
  • The views and controller are less connascent. For example, if the show view wants to display a list of all posts in the side bar, only the view needs to change, not the controller.
  • The code is easier to keep efficient because those database lookups only happen if they are needed. There is little chance of looking up data that is not used by any one. The lookups only happen if the controller or views explicitly request the data.
  • The view code is more reusable. If you want to reuse that partial in another view (say by fully displaying the ten most recent posts in the index view) you can easily do so by rendering it with a post local variable.

Encapsulation is just as good a policy for controllers as it is for models.