Railsのbefore_actionで通常のインスタンス変数をセットするのは、本当にそれが最適解か考えてからやってほしい

before_actionでインスタンス変数を設定するとミスリードが多くなりがちなのでそれに対する話。

（記事製作時）の環境

# bin/rails -v
Rails 6.0.3.1

(前提として)before_actionとは

ActionControllerが提供するフィルタの機能の一種。

before_action は indexやshowなどのアクション定義をしているメソッドに対してそのメソッドを実施する前に実行するメソッドを設定することができる。

before_actionの利用例

Railsガイドでは以下のような記述が例として記載されている

class ApplicationController < ActionController::Base
  before_action :require_login

  private

  def require_login
    unless logged_in?
      flash[:error] = "You must be logged in to access this section"
      redirect_to new_login_url # halts request cycle
    end
  end
end

このメソッドはエラーメッセージをflashに保存し、ユーザーがログインしていない場合にはログインフォームにリダイレクトするというシンプルなものです。「before系」フィルタによってビューのレンダリングやリダイレクトが行われると、このアクションは実行されません。 - Action Controller の概要 - Railsガイド - フィルタ

例示にもある通り、アクションの処理の実施前に何かしらのフィルタリングをしたい場合などに利用することが例示として挙げられる

今回考えたいケース

今回考えたいのはControllerをScaffoldで作ったときにできる以下のような構成

class BooksController < ApplicationController
  before_action :set_book, only: [:show, :edit, :update, :destroy]

  # GET /books
  # GET /books.json
  def index
    @books = Book.all
  end

  # GET /books/1
  # GET /books/1.json
  def show
  end

  # GET /books/new
  def new
    @book = Book.new
  end

  ######中略#######

  # DELETE /books/1
  # DELETE /books/1.json
  def destroy
    @book.destroy
    respond_to do |format|
      format.html { redirect_to books_url, notice: 'Book was successfully destroyed.' }
      format.json { head :no_content }
    end
  end

  private
    # Use callbacks to share common setup or constraints between actions.
    def set_book
      @book = Book.find(params[:id])
    end

    # Only allow a list of trusted parameters through.
    def book_params
      params.fetch(:book, {})
    end
end

この記述では before_action にてインスタンス変数をsetしている。この点が微妙に厳しくなる場面があるので、可能であれば避けたい

before_actionでインスタンス変数のsetをすると何が問題なのか

大きく2つ問題がある

インスタンス変数の入るタイミング見えづらくなる

show などは before_actionと記述が近いのである程度見通しとして記述を把握しやすいが、destroyの記述はとおいので記述が遠くなるので、事前に行われる記述としてはキャッチがしづらくなる。

各action内で使われない値であれば問題はないのだが、だいたいはsetしたいものはactionの処理で使うようなデータ（Modelの取得、など）のことが多いので、各actionでインスタンス変数名を書きたい場合にbefore_actionで定義したメソッドの中まで見ないといけなくなる。

インスタンス変数の値の変更がわかりづらくなる

各actionでインスタンス変数を利用したい場合にどのようなデータが定義されたかなどを見る必要があるのと、action単体で見るとインスタンス変数が突然登場したりするケースがあるので、ちゃんと読み解かないとbefore_actionでセットされていることが気付けずデータの代入を読み違う可能がある。

また、インスタンス変数のため普通にprivateメソッドなどで値の変更を行っているとより内容がわからなくなる。

どう書くといいのか

ではどうやって書くといいのかというと、普通に各action内で呼んであげればいい

class BooksController < ApplicationController

  # GET /books
  # GET /books.json
  def index
    set_book
    @books = Book.all
  end

  # GET /books/1
  # GET /books/1.json
  def show
    set_book
  end
  
  # 中略
  
  private
    def set_book
      @book = Book.find(params[:id])
    end
  # 後略
end

更にいうのであればインスタンス変数のセットを各アクションでやってあげたほうが明快になる。

class BooksController < ApplicationController

  # GET /books
  # GET /books.json
  def index
    @books = Book.all
  end

  # GET /books/1
  # GET /books/1.json
  def show
    @book = load_book(params[:id])
  end
  
  # 中略
  
  private
    def load_book(id)
      Book.find(id)
    end
  # 後略
end

コード日進月歩

しんくうの技術的な小話、メモ、つれづれ、など