🎼
別のメソッド内で定義したインスタンス変数を使うな高校校歌
初めに結論
別のメソッド内で定義したインスタンス変数を使うな
問題提起
複数箇所で再利用したい処理や、あるメソッドの可読性を低下させるような長い処理を別のメソッドに抽出したい時ってあると思います。
しかし、これはあまり良くないメソッドの抽出だなと思うケースがあったのでメモ。
class HogesController < ApplicationController
def show
set_hoge
response = {
hoge: @hoge,
}
render json: response, status: :ok
end
def create
# 省略
end
# その他いろいろなメソッド
private
# いろいろ
def set_hoge
@hoge = Hoge.find(params[:id])
end
# いろいろ
end
サンプルコードにおけるHogesController
のshow
アクションを見ると、
レスポンスの一部にどこで定義されたのか分からない@hoge
が使われています。
で、コード全体を見てみると、set_hoge
メソッド内で定義されていたんだねってことが分かります。
エディタの検索機能を使えば現状問題ないことは分かりますが、
もし今後、機能の追加やリファクタリングをする中で、set_hoge
の呼び出し位置とresponse
の定義位置の順番が逆転してしまったら終わりです。
対応
サンプルコードの例だと、以下のように修正しておくと
以後の変更の際にデグレが起こりにくいと思います。
def show
@hoge = fetch_hoge
response = {
hoge: @hoge,
}
render json: response, status: :ok
end
private
def fetch_hoge
Hoge.find(params[:id])
end
嗚呼〜
嗚呼〜別のメソッド内で定義したインスタンス変数を使うな高校〜
Discussion