コードレビューをしていていた日のこと。あるテーブル(users)からidで絞り込んでレコードを1件取得する際に、以下のようなコードが頻出していることに気づきました。
User.find_by!(id: params[:user_id])
処理の内容としては、idカラムでusersテーブルから該当のレコードを1件取得するというものですが、存在しなかった場合に例外が発生するようにfind_by!を使っています。
不思議なのは「idで絞り込んで、存在しなければ例外を発生させたい」というのはfind関数で事足りる処理なのに、あえてfind_by!(id: params[:user_id])としている点です。
「find関数を知らないのかな」と思ったのですが、railsチュートリアルにも登場する関数なので知らない可能性は低いです。もしくは「find関数だとパッと見て何で絞り込んでいるのかが分かりにくい」からではないかとも考えられます。
言いたいことは分かるのですが、先程のコードのようにparams[:user_id]のような値が引数に指定されているのであれば、idで絞り込んでいるのは自明なため、やっぱりfind関数でいいじゃんと思ってしまいます。
「idと一緒に他のカラムも検索する可能性がある」というのはidが識別子として仕事してないので、論外です。
また、rubocopのスタイルガイドにもidの絞り込みを行う場合は、findを使うことが推奨されています。
# bad User.where(id: id).take! # bad User.find_by_id!(id) # bad User.find_by!(id: id) # good User.find(id)
※find_by_id!なんてものがあるの初めて知りましたが、すでに廃止された関数のようでした。
細かいことですが、自明なことをあえて別の書き方をする必要性について考えさせられました。結論としては、僕の意見がどうかではなく、rubocopが推奨しているものが模範解答でいいかなと思います。なので、これからもidの絞り込みにはfind関数を使っていきます。
