before_actionでインスタンス変数を設定するとミスリードが多くなりがちなのでそれに対する話。
(記事製作時)の環境
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
end
end
end
このメソッドはエラーメッセージをflashに保存し、ユーザーがログインしていない場合にはログインフォームにリダイレクトするというシンプルなものです。「before系」フィルタによってビューのレンダリングやリダイレクトが行われると、このアクションは実行されません。 - Action Controller の概要 - Railsガイド - フィルタ
例示にもある通り、アクションの処理の実施前に何かしらのフィルタリングをしたい場合などに利用することが例示として挙げられる
今回考えたいケース
今回考えたいのはControllerをScaffoldで作ったときにできる以下のような構成
class BooksController < ApplicationController
before_action :set_book, only: [:show, :edit, :update, :destroy]
def index
@books = Book.all
end
def show
end
def new
@book = Book.new
end
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
def set_book
@book = Book.find(params[:id])
end
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
def index
set_book
@books = Book.all
end
def show
set_book
end
private
def set_book
@book = Book.find(params[:id])
end
end
更にいうのであればインスタンス変数のセットを各アクションでやってあげたほうが明快になる。
class BooksController < ApplicationController
def index
@books = Book.all
end
def show
@book = load_book(params[:id])
end
private
def load_book(id)
Book.find(id)
end
end
参考リンク