Rails

railsでレビューしてもらう前に気をつけること

レビューしてもらう際に気をつけた方がいいことをメモしておく

each

  • mapで代用できないか考えてみる
  • mapよりも、each_with_objectinjectの方が良いか考えてみる
  • trueやfalseの判定であれば、all?やnone?、any?、one?が使えないかを考えてみる
  • ハッシュから特定のキーを取得するのであればslice、削除するのであればexceptが使える

unless

sample.rb
# ifで書いた方が可読性が高いので置き換えられないかを考えてみる
unless a == b

if a != b

# 条件が複数あった場合は、ifを使った方が可読性は高いので使わないようにする
unless a.present? && b.present?

if a.blank? || b.blank?

# unless~else は見通しが悪いので使わないようにする
unless user
  ...
else
  ...
end

if user.blank?
  ...
else
  ...
end

トランザクション

  • 一連のロジックで、複数の更新処理がある場合は利用
  • ロールバックされるように、例外が出るようになっているか(ex. save!update!)
  • そもそもActiveRecord::Base.transactionで括らずに、saveやupdate一発でまとめられないかを検討

破壊的メソッドを多用しない

意図せず破壊的メソッドが呼ばれることによって、バグ含むコードを生む可能性を下げるため

hash = { hoge: 111 }

hash.merge!(hoge2: 222)
とするよりも
hash = hash.merge(hoge2: 222)

バッチ処理

  • 大量のレコード数を扱うことが多いので、オブジェクトの生成コストを考慮して、find_eachを利用する

同じロジックをまとめることができないか

  • 同じクラス内であればprivate・インスタンス・クラスメソッド化するかscope使えないか検討する
  • 別クラスであればconcernにまとめる
  • 継承元が同じであれば、継承元にまとめる(ex. application_controllerなど)
  • view側の表示ロジックであれば、helperdecoratorの利用を検討してみる。または、partialとして切り出せるか検討する。
  • validationであれば、独自バリデーターを作る

migrationファイル

  • 必須項目であればnull: falseは入っているか
  • index必要か
  • uniq制約必要か
  • マイグレーション実行後にロールバックできるか
  • varcharでいいのか(charにした方が良いか)
  • カラムの位置の調整が必要であればafterオプションなどで調整する

モデル

  • migrationファイルの定義と同じvalidationが入っているか
  • spec書いているか
  • DBに保存するデータなのかを考える。定数として定義したり、config、またはredisなどを使えるかどうか検討してみる
  • ActiveRecordが必要なのか?ActiveHashで代用できるか考えてみる
  • rails consoleでメソッドの動きを確認してみる

ビュー

  • キャッシュが必要か?フラグメントキャッシュやredisにキャッシュさせるなど検討してみる
  • 共通化するためにパーシャル利用したほうがいいか
  • メタタグに必要な情報入っているか、OPGを忘れてないか
  • アクセス解析しているのであれば、パラメーター付け忘れてないか
  • パーシャルに渡している引数の取扱いは注意する。引数を削除したことにより、他の場所で参照できなくなり落ちることがあるため。local_assignsを使うようにすることを検討する。

WEBrickの利用

  • pryを使ってデバッグしてバグを減らす
  • ログを確認して動きを確認する

その他

  • rubocopを導入して、細かい指摘を受けないようにする(不要なスペースがあったり、チームのコーディングルールに即していないとか、指摘する方も面倒になるので)
  • gemを入れる時は活動状況を確認してみる。(バグった時に質問できない、解決できない負債を減らすため)
  • jsを修正した祭にはchromeのconsoleでエラーが出てないかチェックする
  • Gemfile.lockの末端のbundlerのバージョンは各環境で統一されているか確認する