Rails Anti-Pattern: Setting View Variables In Before Actions

I see a lot of the following pattern in Rails projects:

class ProductsController < ApplicationController  
  before_action :assign_post, only: %i(show edit)

  def index
    @posts = Post.paginate(params[:page])
  end

  def show
  end

  def edit
  end

  private

  def assign_post
    @post = Post.find(params[:id])
  end
end  

Most people do this, because for them @post = Post.find(params[:id]) is duplication. Having assign_post is more DRY.

I don't buy the duplication argument. The definition for DRY is:

“Every piece of knowledge must have a single, unambiguous, authoritative representation within a system."

Unfortunately, most people confused it with - "I don't want to have the same sequence of characters more than once". David Chelimsky have an excellent talk on that subject.

But, my biggest issue with the approach is that it "hides" all variables passed to the view. Especially if there are only or except passed to before_action. Then you have to become an interpreter to figure out what gets into the view. I can recall several situations when during a code review I found code passing unwanted variables to a view.

I prefer to think the following, when I read a controller:

ok, show sends @post and @comments to the view

Instead of:

So, if show is called then assign_posts is called before it, which sets @posts, then show is not included in this other action filter, but is not excluded for this third one...

Also, it is easier for a developer to spot, that too many variables are passed to the view if all of them are listed together.

Because of that, I prefer to use the following pattern:

class ProductsController < ApplicationController  
  def show
    @post = find_post
  end

  def edit
    @post = find_post
  end

  private

  def find_post
    Post.find(params[:id])
  end
end  

In this way, the duplication is moved into finder methods. So if I have to add a scope like visible to Post.find, I just need to apply it in find_post.