Help us understand the problem. What is going on with this article?

【Rails】コーディング時に気をつけるべきこと

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.html.slim
- 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について調べてみた」です。
お楽しみに!

gggk
tabelog
お店選びで失敗したくない人のためのグルメサイト「食べログ」を開発しています
https://tabelog.com
Why not register and get more from Qiita?
  1. We will deliver articles that match you
    By following users and tags, you can catch up information on technical fields that you are interested in as a whole
  2. you can read useful information later efficiently
    By "stocking" the articles you like, you can search right away