こんにちは、ジョンです。
今回は、Rails初心者の方がついやってしまいがちな書き方について書いてみることにしました。
あくまでも「何が何でもこう書くべきだ！」という趣旨ではなく、
「こんな書き方も出来ますよ」という内容ですので、
実際にコーディングしているチームの方針やレビュアーによっては好まれない場合もあります。
そこは、臨機応変に対応をお願いします。
ファイルの行数を減らすコツ
まず、皆さんの~~~~Controller.rb、肥大化していませんか？
Railsのコーディング規約に準拠しているかを判断する RuboCop のデフォルト設定によると、
1つのmoduleにつき最大100行、1method（def〜end）につき最大10行、となっています。
………私も思います（ｺﾗ
ただ、行数が多いコードというのは、
それだけ読み手にかける負荷が多いコードと言い換えることが出来ます。
（もちろんめっちゃキレイに書いてある場合もあるので、一概にこうとは言えませんけどね）
そのため、極力簡潔にかつ分かりやすくソースコードを書く工夫、というのが重要です。
そんなわけで、まずはソースコードを減らせるコツ、というのを紹介したいと思います。
後置 if/unless を使いこなそう！
早速、例として以下のコードを見てみましょう。
# パラメータで渡された情報をモデルに保存する
def create
user_params = params[:user]
# パラメータの中にメールアドレスはあるか？
if user_params[:email].present?
# 受け取ったメールアドレスは正しいフォーマットか？
if validate_mail_format?(user_params[:email])
user = User.new(email: user_params[:email])
# UserモデルのValidationに準拠しているか？
if user.valid?
# userを保存して、200レスポンスを返す
user.save
response_success and return
else
# 400エラーを返す
response_bad_request and return
end
else
# 400エラーを返す
response_bad_request and return
end
else
# 400エラーを返す
response_bad_request and return
end
end
（コメント抜き18行）
else が大量で、どれがなんの else なのか分かりにくいですね。
そんな時は、 後置 if/unless を使ってみるといいでしょう。
改善したコードは以下のとおりです。
# パラメータで渡された情報をモデルに保存する
def create
user_params = params[:user]
# パラメータの中にメールアドレスがなかったら400エラーを返す
response_bad_request and return unless user_params[:email].present?
# 受け取ったメールアドレスは正しいフォーマットではなかったら400エラーを返す
response_bad_request and return unless validate_mail_format?(user_params[:email])
user = User.new(email: user_params[:email])
# UserモデルのValidationに準拠しているか？
response_bad_request and return unless user.valid?
# userを保存して、200レスポンスを返す
user.save
response_success and return
end
（コメント抜き8行）
このように書くと、どんな時に400エラーを返しているのかがすぐわかり、
コードレビュー者としてもとても見やすいコードになったと思います。
メソッドの行数としても、上の書き方に比べて、10行のダイエットに成功しました。
ただし、後置 if/unless も多用すればいいというわけではありません。
例えば、
response_bad_request and return unless (current_user.admin? and current_user.has_edit_permission? and user.valid? ) and (user.name.length > 10 or user.age > 20） and …
なんてやられた日には、
ログイン中のユーザーが管理者 かつ
編集権限を持っている かつ
Userモデルに準拠していて かつ … ではない時？
とまぁ酷い混乱の原因になります。
否定の否定を重ねるのはやめましょう…
こういう時は、必要な条件をまとめて、それに準拠していれば true、
条件を満たしていなければ false を返すような private_method を作成し、
それを unless に繋げてあげるといいかもしれません。
また、後置 if/unless はどうしても、1行の文字の長さが長くなってしまう傾向にあるので、
そういった意味でも、何でもかんでも後置でとするのは良くないかもしれないですね。
safe navigation operator（&.）を使いこなそう！
次の例を見てみましょう。
# 全著者を1件ずつ確認する
Author.all.each do |author|
# その著者は出版した本を持っているか？
if author.books.present?
# その著者は今日発売の本があるか？
if author.books.where(release_date: Date.today).present?
：
：
end
end
end
各著者（Author）に紐付いている出版した本（Book）の中で、
今日発売の本はあるか？という処理です。
これを、 &. を使うと、以下のように修正できます。
# 全著者を1件ずつ確認する
Author.all.each do |author|
# その著者は今日発売の本があるか？
if author.books&.where(release_date: Date.today).present?
：
：
end
end
&. とは、もしオブジェクトが存在しない（nilの場合）でも、
問題なくscopeやメソッド、SQLを呼び出せる演算子で、
上記の例の場合、 author.books が nil だったとしても、エラーにはなりません。
author.books が nil の場合は、
author.books&.where(release_date: Date.today)
が nil となるので、
nil.present? となり、if文としては falseと判定されます。
&:method を使いこなそう！
今度は、ループに関連する内容です。以下の例を見てみましょう。
# アクティブユーザーの名前を取得する
user_names = User.where(status: :active).map do |user|
user[:name]
end
このように、ある特定の要素を取得したい、という場合、以下のように変えることが出来ます。
# アクティブユーザーの名前を取得する
user_names = User.where(status: :active).map(&:name)
余談ですが、更にRailsの場合、上記の例のように、データベースから取得したデータの中から
ある特定のカラムのみを使用する場合、map ではなく、 pluckを使うと更にGoodです。
# アクティブユーザーの名前を取得する
user_names = User.where(status: :active).pluck(:name)
これは、実際に rails c で実行してみると一目瞭然なのですが、
pry(main)> User.all.map(&:id)
User Load (4.0ms) SELECT `users`.* FROM `users`
=> [1]
pry(main)> User.all.pluck(:id)
(3.1ms) SELECT `users`.`id` FROM `users`
=> [1]
このように、必要なカラムのみを取得してくるSQLに変わるため、処理速度が早くなります。
これらが使いこなせるようになると、一気に脱初心者なコードな気がします。
あくまでも個人的な主観です。
find, find_by, where を使い分けよう！
皆さんは find, find_by, whereの違いを理解していますか？
いずれも検索キーとして使えるメソッドですが、実は色々と仕様が異なるのです。
find
まず find ですが、基本的に id でのみ検索可能で、
該当のレコードがない場合は、エラーが返ってきます。
pry(main)> User.all.pluck(:id)
=> [1]
pry(main)> User.find(10)
=> ActiveRecord::RecordNotFound: Couldn't find User with 'id'=10
find_by
次にfind_byは特定のカラムを指定して検索が可能です。
もちろん、カラムは複数選択することも可能です。
また、find と異なり、該当のレコードがない場合でもエラーにならず、 nil が返ってきます。
しかし、このメソッドで取得できるレコードは、最初にヒットした1件のみです。
pry(main)> User.all.pluck(:id)
=> [1]
pry(main)> User.find_by(id: 1)
=> #<User:0x0000aaaaea1a1a18
id: 1,
name: "ジョン",
:
>
pry(main)> User.find_by(id: 1, name: "hogehoge")
=> nil
where
最後に where ですが、find_by同様、カラムを指定して検索をかけることが出来ます。
また、find_by と同様、該当のレコードが存在しない場合でもエラーにはならず、 nil が返ってきます。
pry(main)> User.find_by(id: 1)
=> #<User:0x0000aaaaea1a1a18
id: 1,
name: "ジョン",
:
>
pry(main)> User.where(id: 1)
=> #<ActiveRecord::Relation [#<User:0x0000aaaaea1a1a18
id: 1,
name: "ジョン",
:
>]>
ただし、find_by と違い、該当した全てのレコードを取得します。
そのため、find_by の場合、特定のオブジェクトが取得できるので、そのままメソッドを呼ぶことが出来ますが、
whereの場合、仮に取得できたレコードが1件だったとしても、そのままメソッドを呼ぶことは出来ません。
pry(main)> User.find_by(id: 1).name
=> "ジョン"
pry(main)> User.where(id: 1).name
NoMethodError: undefined method `name' for #<ActiveRecord::Relation [#<User:0x0000aaaaea1a1a18
id: 1,
name: "ジョン",
:
>]>
もしメソッドを呼びたい場合は、取得したレコード群の中から、どのレコードを使うかを指定してやる必要があります。
pry(main)> User.where(id: 1).name
NoMethodError: undefined method `name' for #<ActiveRecord::Relation [#<User:0x0000aaaaea1a1a18
id: 1,
name: "ジョン",
:
>]>
pry(main)> User.where(id: 1).first.name
=> "ジョン"
使い分けるコツは？
あくまでも私の考え方にはなりますが、私は以下のように使い分けています。
find
→ 検索したいIDが確実に存在していることがわかりきっている場合。
（正直ほとんど使いません）
find_by
→ 重複したデータが存在しないカラムで検索する場合。
例：email, token, 等
where
→ 該当するデータが複数存在する可能性のある検索をする場合。
例：status, gender, birthday, 等
参考になれば幸いです。
最後に
いかがだったでしょうか？
Railsを始めたばかりの初心者がついやってしまいがちな内容について書いてみました。
というのも、私がRailsを始めてから、実際にコードレビューの際に、
こうしたほうがいいよとよく指摘を受けた箇所に重点を当てて書きました。
実際に、上記のことを意識して書くだけで、かなり読みやすいコードになると思います。
この記事が、どなたかのお役に立てれば幸いです。
