Explaining the rationale behind using memoized helper methods for controller resources

memoized-helper-methods.md

Last year I started playing around with using memoized private helper methods in my controllers instead of the traditional instance variable assigns we see in RoR controllers. Here's an example:

class PostsController < ApplicationController
  helper_method :new_post, :post, :posts
  
  def new; end
  def show; end
  def edit; end
  def index; end
  
  def create
    new_post.attributes = post_params
    
    if new_post.save
      redirect_to post
    else
      render :new
    end
  end
  
  def update
    if post.update_attributes(post_params)
      redirect_to post
    else
      render :edit
    end
  end
  
  def destroy
    post.destroy!
    redirect_to :index
  end
  
  private
  
  def new_post
    @new_post ||= Post.new
  end
  
  def post
    @post ||= posts.find(params[:id])
  end
  
  def posts
    @posts ||= current_user.posts
  end
  
  def post_params
    @post_params ||= params.require(:post).permit(:title, :body)
  end
end

Here's what I like about it:

• Actions #new, #show, #edit, #index have no unique logic and as such, nothing in their methods (in fact, we don't even have to define methods here but I usually define stub methods to make it clear to readers that those actions do exist)
• Actions #update, #create, and #destroy contain only the logic that is unique and important to them
• Avoids hard-to-maintain before_filter :only and :except lists or repetative resource instantiation code
• No more "called method on nil" errors. If you misspell something, you'll get an "undefined method" exception which includes your misspelling.
• No more weird ass Rails magic copying ivars from one context to another

I also got into the habit of abstracting the logic in #update and #create into another private method (not helper) but I've since stopped doing that because I think it adds more indirection than value. Here's an example of that:

class PostsController < ApplicationController
  helper_method :new_post, :post, :posts
  
  def new; end
  def show; end
  def edit; end
  def index; end
  
  def create
    save_or_render(:new)
  end
  
  def update
    save_or_render(:edit)
  end
  
  def destroy
    post.destroy!
    redirect_to :index
  end
  
  private
  
  def save_or_render(action)
    if post.update_attributes(post_params)
      redirect_to post
    else
      render action
    end
  end
  
  def new_post
    @new_post ||= Post.new
  end
  
  def post
    @post ||= posts.find(params[:id])
  end
  
  def posts
    @posts ||= current_user.posts
  end
  
  def post_params
    @post_params ||= params.require(:post).permit(:title, :body)
  end
end

Nice write up.

I do like the benefits.

My concern when seeing this technique is that the bigger the controller gets, the more it leads to private methods doing more than one thing or several methods with similar names that do similar things. And as it happens, it gets harder to follow what actually are we working on at the view.

i.e. a private method doing more than one thing:

  def index
    params[:post_state] ||= "published"
  end

  def drafts
    params[:post_state] ||= "draft"
  end

  def posts
    @posts = if params[:post_state]
      current_user.posts.where(state: post_state)
    else
      current_user.posts
    end.order("published_at DESC")
  end

i.e. with several methods:

  def index; end
  def drafts; end

  def published_posts
    current_user.posts.where(state: 'published')
  end

  def draft_posts
    current_user.posts.where(state: 'draft')
  end

If a controller is complex enough, this would be a pain to sort out and, depending on the view, hard to figure where in those ifs in the first case, or make sure I know which method I'm actually using on the view.

I like using memoized methods for common logic, keep using instance variables (letting rails do its magic), and keep them on the action itself most of the time.

i.e.:

  def index
    @posts = posts.where(state: 'published')
  end

  def drafts
    @posts = posts.where(state: 'draft')
  end

  def posts
    @_posts ||= current_user.posts
  end

I will still get nil on typos from time to time, but at least I know just by looking at the action method what 90% of the things are happening, no ifs, and just private methods like people generally use with a before_filter or common logic.

That is a very good point! I'm guilty of having written methods like the long #posts method from your example in the early days of this technique. I'm actually a fan of the second example (smaller appropriately named methods) but I can see how this could get out of hand with a less CRUD-y controller. In that case I think your last example could be appropriate, but I'd probably still gravitate toward the second.

Thank you for your feedback!

Pedro's comment "it gets harder to follow what actually are we working on at the view" rings really true to me.. Although IMO the assigns magic isn't a much better approach to an explicit interface. Looking forward to Brendan's views gem! (Ask him about what he's been working on in his spare time...)

That was really good explaination and thanks for all the code. You've show many unpredictable things..