🤔
コードを書く時に意識していること【Ruby/Rails】
命名に気を付けるのは大前提で頑張る
その上で、気を付けていることのまとめ
チーム開発している場合はメンバー達で話し合って決める、そこでの合意事項を一番尊重する
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
oreager_load
-
includes
はpreload
になるかeager_load
になるか判断が難しいから
-
- LIKE検索に嫌悪感を持つ
- 遅い
- 時刻が外部から与えるようにする
- テストしやすくなる
- Viewの中でクエリを実行しないようにコントローラ側でロードしておく
- N+1を起こしやすくなる
- 特定の組み合わせの
preload
/eager_load
するメソッドを定義して、モデル内で使う事はしない- コントローラ毎にロードしたいものが微妙に違うので共通化すると不要なデータもロードしてしまう事になりがち
- 複数レコードを更新するときはトランザクションを使う(コントローラでトランザクションを貼る)
- 処理の途中で失敗したら全部なかった事になった方が楽
- トランザクションの中で外部サービスなどを呼ばないように意識する
- トランザクションの最後の方にまとめる事ができると良いがなかなか難しい
- 小数を避ける
- コンピュータは小数が苦手
-
id
はunsigned
でbigint
にする- シャーディングする場合は
ulid
を使うと良いかも?
- シャーディングする場合は
- NULLを許容しない
- 頑張る!
- 外部キー制約を貼る
- デメリットもあるにはあるが回避できなくなったら辞めればいい
Discussion