Webエンジニア2年目の@gggkです。
これまで、コードレビューで色々と指摘を受けてきたのですが、コードを書く上で大切なことなので、コーディング時に気をつけるべきことをまとめてみました。
特に、Rails慣れたての人の参考になればいいなと思います！
#バグを生み出さない
エンジニアやってて怖いのは、リリース後にバグが出ることですね。普段から意識してコーディングすることで、バグを発生させないようにしましょう。
nilチェックをする
大体のバグは、nilが原因となることが多いんじゃないかなと思います。
レシーバーがnilになってしまい、メソッドを呼び出そうとして「NoMethodError」になる。
変数に値が入っていないときは、returnして次の処理をしないようにして対応することができます。ifで分岐してもいいですね。
# restaurantがnilになってしまってもエラーにはならない
restaurant = Restaurant.find_by(id: 1)
return unless restaurant
restaurant.name
または、ぼっち演算子(&.)を使用してレシーバーに定義されていないメソッドを呼び出した場合は、nilを返すようにすれば対応できます。
restaurant&.name
=>nil
##SQL発行される処理を定数にしない
NG
TOKYO = Area1.find_by(name: "東京")
メソッドで呼び出す形にしましょう。
OK
def tokyo
Area1.find_by(name: "東京")
end
##private_constant
定数を定義した際に、そのクラス内でしか使用しない場合は、private_constantを使用して、他のクラスから呼び出されないようにする。
class Italian
PIZZA = "ピザ"
PASTA = "パスタ"
private_constant :PIZZA, :PASTA
end
##partialで受け取る変数を初期化
partialで変数を初期化しておけば、変数の受け渡しを忘れた場合にもエラーにはならない。
(デメリットとしては、逆にエラーに気づけないということもあるので注意)
- users ||= []
- users.each do |user|
= user.name
#パフォーマンス
##pluckを使用する
例えば、DBから都道府県の名前データを取得したい場合に、ActiveRecordモデルをすべて読み込むのではなく、pluckを使用して、必要な名前だけを配列で取得するようにします。
そうすることで、メモリを大量に使わずに済み、速度も早くなります。
NG
prefs = Prefecture.all
prefs.map(&:name)
=>["北海道","青森",...]
OK
Prefecture.all.pluck(:name)
=>["北海道","青森",...]
##n+1問題
includes,preload,eager_loadを使用することで、SQLの発行回数を削減することができます。n+1問題については、@massaaaaanさんの下記記事で説明しているので、どうぞご覧ください。
【Ruby on Rails】N+1問題ってなんだ？
##メモ化
複数回呼ばれる場合に下記のようにすることで、2回目以降は1回目で処理した値を返すことができる。
def countries_link
@_link ||= build_countries_link
end
また、build_countries_linkが、nilになる可能性がある場合は下記のように書くと良い。
この場合は、@_linkが、定義されている場合はその値を返す。
def countries_link
return @_link if defined?(@_link)
@_link = build_countries_link
end
#可読性
読みやすさは大事ですね。次、自分や他の人が見たときに、わかりやすいように記述しましょう。改修がしやすくなり、バグも起こりにくくなります。
##早期リターン
処理しない条件で早めにreturnすることで、複雑にならずに見やすくなります。
def food_genre(food_name)
return if food_name.blank?
return if food_name == "hogehoge"
genre = Food.find_by(name: food_name)&.genre
end
##hashのslice,except
hashから、必要な値だけを取り出したいときに使用する。
paramsの値を取得するときに便利。
hash = { a: "hoge", b: "foo", c: "bar" }
# aとcのみ取り出す
hash.slice(:a, :c)
=> { a: "hoge", c: "bar" }
# c以外取り出す
hash.except(:c)
=> { a: "hoge", b: "foo" }
##index_by
配列を特定のキーのhashにしてくれる。モデルからpluckで値を取得したときになどに、扱いやすくなる。
Food.where(genre: "和食").pluck(:id, :name, :en_name).index_by {|item| item[0] }
=>{1=>[1,"寿司","sushi"], 2=>[2,"鍋","nabe"], ...}
##each_with_object
配列やhashなどのオブジェクトに繰り返し、値を入れていく処理などのときに、初めにオブジェクトを定義する必要がなくなる。
price = { apple: 100, orange: 30, banana: 200, cherry: 10 }
# 100以上のものを調べてキーを配列にする
fruits = price.each_with_object([]) do |(key, val), arr|
arr << key if val >= 100
end
=>[:apple, :banana]
##その他細かい点
- {}の前後にスペースを入れる
- カンマ(,)の後にスペースを入れる
- ロケットハッシュを使用しない
{ a: "hoge", b: "hoge" } # こっちの方が見やすいと思う
{:a=>"hoge",:b=>"hoge"}
- %記法を活用する
%w(apple orange banana cherry)
=>["apple", "orange", "banana", "cherry"]
%i(apple orange banana cherry)
=>[:apple, :orange, :banana, :cherry]
#最後に
今回は、Railsプロダクトのコードチェックで、指摘されがちな点をまとめてみました。
私も、ありがたいことに先輩エンジニア達から多くの指摘をされてきましたが、次コーディングするときには気をつけるようにしています。自分がコードチェックするときにも指摘できるようになっていくので、コードチェック大切ですね。
明日は、@ya-manさんの「Ansibleを最大で25倍高速化するMitogenについて調べてみた」です。
お楽しみに！