In this short rant I will show you how to change most before_action
calls into plain Ruby.
Does this make your eyes bleed?
# 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:
- Side-stepped the need to parse all hooks when trying to understand what hooks a new action should get.
- Eliminated any order-dependence on
@instruction
being available. Just use the memoising private getter, same as in other parts of Rails code. - Reduced the risk of double-auth (in hook and in action itself), it happens.
- Less Rails™, more Ruby ❤️.
Con:
- 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.