Stop abusing before_action

In this short rant I will show you how to change most before_action calls into plain Ruby.

Does this make your eyes bleed?

source

# bad, hook approach class InstructionsController < ApplicationController before_action :load_instruction, only: %i[show edit] def show authorize @instruction end def edit authorize @instruction end private def load_instruction @instruction = Instruction.find(params[:id]) end 

This is a weird antipattern. Hooks are like guard clauses, intended to step in before getting to action itself (authentification, redirects).

Consider Grug's advice - keep code pertaining to a thing in one place. Let's get rid of before_action here and have everyting an action needs in it.

# good, simple memoisation, no verbs in getters class InstructionsController < ApplicationController def show authorize instruction end def edit authorize instruction end private def instruction @instruction ||= Instruction.find(params[:id]) end end 

Discussion

Pros:

1. Side-stepped the need to parse all hooks when trying to understand what hooks a new action should get.
2. Eliminated any order-dependence on @instruction being available. Just use the memoising private getter, same as in other parts of Rails code.
3. Reduced the risk of double-auth (in hook and in action itself), it happens.
4. Less Rails™, more Ruby ❤️.

Con:

1. Admittedly we've not done much about unclear source of @instruction. It's still loaded as a side-effect, but there's a cure for that also - stop using ivars in controllers and pass all needed variables to template explicitly.