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
