https://プログラマが知るべき97のこと.com/エッセイ/DRY原則/
基本的には「達人プログラマー」に書かれている内容の通りに意識することが多い。
自分の経験の中では以下のようなコードもDRYでない、つまり共通化した方がよいのでは?という議論をしたことがある
class UsersController
def show
@user = User.find(params:id)
end
def edit
@user = User.find(params:id)
snip
end
end
というコードを
class UsersController
def show
set_user
end
def edit
set_user
end
private def set_user
@user = User.find(params:id)
end
end
のようにshowメソッドとeditメソッドで共通になっている@user = User.find(params[:id])の部分を切り出す形にするのはどうかという議論。
Railsのgeneratorを使うとこういうコードは生まれがちなのでたまにレビューすることがあるけど、おおむねこういうコードはDRYであるかどうかよりも早すぎる共通化や抽象化のような側面が大きいと思うので議論の進め方としてはそちらの方が適切であると思う。set_userの中身が今後どのような変化を遂げるのかをチームで予想してみるのはよいかもしれない。
- scopeが追加されたとして、それが参照のaction(show,edit,update,destroy)の中でキチンと振る舞いを維持できるか
- 取得するデータの状態に依存した参照にならないか
- set_userの中で分岐が発生するのであれば、各actionに書き下す?書き下さない?
- 特に論理削除の仕組みが生まれた時など
あたりを話してみるとよいかもしれない。
個人的には最初から書き下している方が読みやすい。数十行程度であれば確かに1画面に収まるかもしれないけど、複雑な処理が追加されて set_user が画面外に行ってしまうとカーソルの移動が発生してしまうのが煩わしいと感じる