Stop abusing before_action

#rails #controller

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

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.