Zenn
🤔

コードを書く時に意識していること【Ruby/Rails】

に公開
Rails
Ruby
tech

命名に気を付けるのは大前提で頑張る
その上で、気を付けていることのまとめ
チーム開発している場合はメンバー達で話し合って決める、そこでの合意事項を一番尊重する

else / elsif を避ける

  • 早期リターンを使う
  • else / elsif を使うとネストが深くなる
  • ネストが深くなるとコードを読むのが大変になる
    • else がきた時に if の内容を別領域のメモリに待避してから読む進めるので脳内のリソースを消費している感覚がある
def hoge
  if hogehoge?
    do_something1
    do_something2
  elsif fugafuga?
    do_something3
    do_something4
  else
    do_something5
    do_something6
  end
end

は

def hoge
  if hogehoge?
    do_something1
    do_something2

    return
  end

  if fugafuga?
    do_something3
    do_something4

    return
  end

  do_something5
  do_something6
end

のように書く（が、ここで終わらせない）

if の中身をメソッドに切り出す

  • 早期リターンを使うと if のみになるけど中身が長いと読みにくい
    • 処理の塊に名前を付けられる事もまずまずあるから切り出す
def hoge
  return do_something_xxx if hogehoge?
  return do_something_yyy if fugafuga?

  do_something_zzz
end

private

# この例だと戻り値が変わっているので必要に応じて変える
def do_something_xxx
  do_something1
  do_something2
end

def do_something_yyy
  do_something3
  do_something4
end

def do_something_zzz
  do_something5
  do_something6
end

条件式での && と || を避ける

  • 条件式はバグを生みやすいから特に気を付ける
  • 積極的にメソッドに切り出す
    • a && b くらいは許容しているが2つ以上は基本的に使わない
    • a && !b など否定が入ると一瞬わからなくなるのでその場合は切り出す
if obj.present? && (obj.value1 > 0 || obj.value2 > 0)
  xxx
end

みたいなコード。

&& と || がとにかく怖いから以下のようにリファクタリングする

if hogehoge?
  xxx
end

...

def hogehoge?
  # unless obj.present? でも良いが、unlessよりはifを使う
  return false if obj.blank?
  return true if obj.value1 > 0
  return true if obj.value2 > 0

  false
end

数学で習った「ド・モルガンの法則」が役に立つケースがある

booleanを返すメソッド / 変数は肯定的な名前を付ける

  • disabled? とか invalid? とかは否定的
    • disabledみたいなシンプルな例の場合はあまり違和感を感じないが
  • enabled? とか valid? が肯定的な名前
if !disabled?
end

みたいに否定(!)と否定(disabled)が重なると読みにくい

unless disabled?

unless を使うと!を消せるのだが「無効じゃない」よりは「有効である」の方がわかりやすい

if enabled?

配列の命名

  • 複数系の名前を付ける
item = []
item << 1
item << 2

return item

より

items = []
items << 1
items << 2

return items

ハッシュの命名

Railsを使っていると良く使う以下のようなハッシュの命名

# group_byとかでもいい
xxx = User.all.index_by(&:id)

# { 1 => user1, 2 => user2, ...} みたいな結果。

上記の場合は user_hash って命名する人もまずまず見るけど

users_by_id と名付ける
複数系_by_キーの名前 というルール

変数名が長くなるけど…。

user_ids.each do |user_id|
  user = users_by_id[user_id]
  ...
end

みたいな感じ

クエリの組み立てが発生するときの一時変数の名前

scoped という名前を使っている

def hogehoge(name = nil)
  scoped = User.all
  scoped = scoped.where(name: name) if name.present?
  scoped = scoped.order(id: :asc)
end

ActiveRecord::Relation を取り扱っているんだぞと意識する為に scoped を使っている

その他、細かいところ

ひとまず、列挙しておく

  • find_by.present? ではなく exists?
    • インスタンス生成されない exists? の方が良い
  • paranoia 使わない
    • 普通にレコードを消した方が楽。deleted_at を考慮したインデックス張れるならば使っても良いけど人類にはそれは厳しい
  • validates_uniqueness_of 使うならばユニークキー制約もちゃんと設定する
    • バリデーションだけで本当のユニークは担保できない
  • all.each ではなく find_each
    • all.each だと全部メモリに載ってしまう
  • Time.now ではなく Time.zone.now
    • タイムゾーン情報大事
  • Rails.env.xxx? は使わずに Settings などに押し込める
    • バグの温床
  • includes ではなく preload or eager_load
    • includes は preload になるか eager_load になるか判断が難しいから
  • LIKE検索に嫌悪感を持つ
    • 遅い
  • 時刻が外部から与えるようにする
    • テストしやすくなる
  • Viewの中でクエリを実行しないようにコントローラ側でロードしておく
    • N+1を起こしやすくなる
  • 特定の組み合わせの preload / eager_load するメソッドを定義して、モデル内で使う事はしない
    • コントローラ毎にロードしたいものが微妙に違うので共通化すると不要なデータもロードしてしまう事になりがち
  • 複数レコードを更新するときはトランザクションを使う（コントローラでトランザクションを貼る）
    • 処理の途中で失敗したら全部なかった事になった方が楽
  • トランザクションの中で外部サービスなどを呼ばないように意識する
    • トランザクションの最後の方にまとめる事ができると良いがなかなか難しい
  • 小数を避ける
    • コンピュータは小数が苦手
  • id はunsigned で bigint にする
    • シャーディングする場合は ulid を使うと良いかも？
  • NULLを許容しない
    • 頑張る！
  • 外部キー制約を貼る
    • デメリットもあるにはあるが回避できなくなったら辞めればいい

Discussion