レビュアーの負担を少しでも減らすために開発で気をつけてること

お仕事でレビューをしてもらっているときに、
「ああ、またここ指摘されたな」
「ここ他の人が指摘されてたような気がする」
みたいなことが何度かあり、このようなミスを減らしたい
と思ったことはありませんか？
私は毎日のように思っておりますT^T（激弱）
ただ、気を付けていたとしても、忙しい時や仕様に集中しすぎてしまうと、思わず質の低いPRを出してしまい、レビュアーの負担を増やしてしまいます。

なので自分は今まで自分や他の人がレビューで指摘されてきたことをnotionにまとめており、PRを出す前に軽くでも確認するようにしています。
これをきちんとした後とそうでない時では指摘率が全然違うなと最近思っています。
「はい？こんなん当たり前やん。」みたいなことも沢山書いてますけど、当たり前なことを当たり前にできるように常にしていけるようになりたいなと思って書いています。
今回はそれをいくつか紹介します。（railsを使用しています）

綺麗に書くために見直すべきところ
PR作成で気をつけること
最後
おまけ

綺麗に書くために見直すべきところ

データと変数名を合わせる

# 例:
user = Users.where(user: user_id) # ❌
users = Users.where(user: user_id) # ⭕️

whereは複数のレコードを返す可能性があるため、変数名を複数形にするべきです。
複数のユーザーが返ってきているのに、受け取る変数がuserだと1人しか返ってきてないの？となってしまいます。

メソッド名と中の実装は合わせる

# 例:
# ❌　
def close_request(request)
  # DB更新
end

def need_create?
  〜〜
  close_request(request)
　　　　〜〜
end

# ⭕️
close_request(request)
def need_create?
  #  DB更新
end

意味のある単位でメソッドを分けて、それぞれのメソッドは綺麗になっていることは
多いんですけど、組み合わせたときにおかしいところが出てきてしまうことがありました。上の例だと、need_create?というメソッドの中でclose_requestが呼び出されているので、need_create?というメソッド名からDBが更新されたことがわからないため、分ける必要があります。

mapとeachの使い分けを考える：意図に合わせた効率的な使用法

# eachの使用例、このコードは各数値を2倍にして出力しますが、新しい配列を作成しません。
numbers = [1, 2, 3, 4, 5]
numbers.each do |number|
  puts number * 2
end

# mapの使用例、このコードは新しい配列doubled_numbersに各数値を2倍にした結果を保存します。
numbers = [1, 2, 3, 4, 5]
doubled_numbers = numbers.map do |number|
  number * 2
end
p doubled_numbers

# map!の使用例、このコードはnumbers配列自体を更新し、各数値を2倍にします。
numbers = [1, 2, 3, 4, 5]
numbers.map! do |number|
  number * 2
end
p numbers

mapは配列を返すメソッドなので、繰り返し処理だけならeachを使うのが👍
mapは繰り返し処理をした結果を配列で保存するときに使いますが、
ただ回すだけなのにmapの方をよく使うので使い分けを考えずに使用してしまうことがあると思います。
何をしたいのかを考えて使い分けると良さそうです。
また、元のデータをいじる場合はmap!を使いましょう。ただ、map!は元データを破壊的に変更するので、注意しましょう。

変数名をパスカルケースやスネークケースなど決まっているケースに揃える

この命名規則は一般的なものですが、キャメルケースとパスカルケース、
スネークケースとケバブケースはそれぞれ似ているため、間違えて書いてしまうことがあると思っています。最終的に命名規則をきちんと見直してPRをあげましょう！

以下、命名規則の説明参考
https://www.wakuwakubank.com/posts/804-it-naming-convention/

マジックナンバーをやめて、enumをきちんと使おう。

# 例:
joins(:hoge).where(hoge_fuga: { fuga_id: 1 }) # ❌　

enum hoge: {
 aaa: 1,
 bbb: 2
}
joins(:hoge).where(hoge_fuga: { fuga_id: Hoge.hoges[:aaa] }) #　⭕️

マジックナンバーは次にコードを見た人がわからなくなるのと、変更に弱いので
enumで定義をしましょう。

何回も同じメソッドを通る場合は、変数一つにまとめてデータ渡す

# 例:
# ❌　
def hoge()
  aaa()
  bbb()
end

def aaa
  search_users
end

def bbb
  search_users
end

def search_users()
  Users.where(id: 1)
end

#　⭕️
def hoge()
  users = search_users
  aaa(users)
  bbb(users)
end

def aaa(users)
  # 何か処理
end

def bbb(users) 
  # 何か処理
end

def search_users()
  Users.where(id: 1)
end

何回も同じメソッドを通って、取得するデータが同じ場合は変数にまとめて必要なところに渡した方が、余計にクエリを回さないので、できるだけまとめると良さそうです。

読みやすい＆変更に強いコードにする

例：
#　❌　
rows = rows.map(&:reverse)
rows = rows.map { |row| row.push(nil, nil, nil, "ほげーた")　}

#　⭕️
rows = rows.map do |row|
  [
    row[1],
    row[2],
    nil,
    nil,
　　　　　　　　nil,
    "ほげーた"
  ]
end

便利メソッドで簡単にかけても、分かりずらい場合は愚直に配列に書いた方が
パッと見てどんなデータが表示されているのかわかるので、読みやすいし、
変更に強い。

xxx_at はdatetimeフィールド、xxx_on はdateフィールド。更新日時、作成日時は「updated_at」「created_at」

create_table :users, force: :cascade do |t|
  t.integer :hoge_id, comment: "hogeID"
  t.datetime :expired_at, comment: "有効期限" # ⭕️
  t.datetime :expired_on, comment: "有効期限" # ❌

  t.datetime :created_at, comment: "更新日時"
  t.datetime :updated_at, comment: "作成日時"
end

これは規約にありますが、時々適当に名前をつけてしまうことがあるので、データとカラム名をきちんと揃える必要があります。
この辺の規約はドキュメントに
色々乗っているので全て確認しておくといいと思います。
https://railsdoc.com/page/configuration

PR作成で気をつけること

テスト結果・テスト項目を書く時、カテゴリーに分ける

カテゴリーに分けられる場合は、タイトルをつけて分かりやすくします。ただ単に羅列するだけでは分かりにくい場合があるため、より分かりやすくするための工夫が必要です。

カラム更新やテーブル追加した場合にER図を載せる

カラムの更新や、テーブルの追加をした場合、ER図を添付すると非常に親切です。コードを見て理解できることもあるかもしれませんが、全体像を把握するためにも、プロジェクトマネージャが見るためにも有用です。私自身も、可能な限りER図を添付するようにしています。大きな変更がある場合は特に注意して添付します。

commitは意味ある塊で分ける

commitは意味のあるまとまりで分けるべきです。
lintやtestが落ちたときは直前のcommitに含めると、不具合等の調査が容易になります。そのため、コミット単位やメッセージには注意を払います。

git commit --amend --no-edit
git push -f origin hogehoge/hugahuda

最後

今回書いたことは一般的なことかもしれませんが、忙しさから見逃してしまうこともあるので、PR作成後にこれらを確認することは大事だと思います。私自身も、他人のレビューを見たり、自分のレビューを受けたりしながら、空いた時間に自分のレビューチェックリストに追加しています。同じような悩みを持つ人は、レビューチェックリストのようなものを作成すると良いでしょう🤗

（ちなみに、久しぶりにブログを書いたので書き方を忘れてしまっていました🙃ブログを書くのは楽しいけれど難しいですね）

おまけ

レビュー側
レビューの際には[imo]や[nits]などの項目を意識して使うのがいいかなと思っています。
それと、やっぱりいい実装にはコメントを付けて、ｲｲﾈ👍👍をするのはとても大事だと思っているので、意識してしています。
lgtmoonは時々使うようにしています。賑やかし大事かなと思って笑

この方の記事はとても分かりやすかったです😊
https://note.com/su_k/n/nf23ab5c6dba2

[must]:絶対直して欲しい   ->　ちょっと強めなのでできるかぎり言い方に気を付ける
[imo]: 自分ならこうします☺️
[nits]: 細かい指摘・軽い修正
[q]: 質問したいときにつける