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.