コードレビューで指摘されないために、コードレビューを知ろう

おおまかに

• コードレビューはどういった観点で見るか
• 実装時もコードレビューの視点を持って作ろう

観点

• 変更意図が要求に沿っているか
  • チケット番号、issueの番号を追加する
  • こんな機能がほしい、こういう風にしたいなど開発には必ず「要望・要求」が存在する
    • 要望・要求を元に機能要件に落ちたものが設計の前段階としてある
    • これをどこまでイメージしきれているかが重要
  • NG: 要件を鵜呑みにし、理解が浅い
  • OK: ユーザー視点で要求や要望まで立ち返り、要件を認識する
• モデリングが妥当か
  • モデルによって意図が表現できているか
• 仕事が適切な粒度で明確に切り分けられているか
  • 単一責任原則
• 意図のない共通化がなされていないか
  • 凝集度と結合度
• わかりやすい名前がつけられているか
  • 名付けの失敗は仕事の失敗
    • そのコードの役割を説明出来ていない
  • 既存のコードがすでに……ということもある
    • 改善できそうな道筋について議論できるといい
    • 振り返りの議題や改善チケットを切ってコメントをつけておく
• 仕事にあったインタフェースになっているか
  • テストを書いたときに素直なコードになっているか
  • 例外の取り扱いや、アウトプット先を変えたときに無理なく使えそうか
• 実装が妥当か
  • プロダクションでの性能が意識されているか
    • 逆に無意味に性能を追ってコードが読みづらくなっていないか
    • 読みにくいワンライナーより、素直な複数行
  • アルゴリズムに誤りがないか
    • とくに外部とのやりとりを伴う場合、エッジケースがテストされているか
      • 境界値周りのテスト実行エビデンスを付ける
  • 異常が発生したときにデバッグ・調査しやすいか
    • エラー（ログ）に必要な情報が含まれているか
  • 既存のコードが今回の意図に合わせて変更されているか
• 読み手の負荷が高くないか
  • コメントが書かれているか
  • ドキュメントへのリンクがあるか
  • 読みづらいコードになっていないか
    • やむなくそうなっている場合はコメントがあるか
      • Pull Request のコメントで補足しない、コメントに書く（コードを読めばわかるようにする）
  • 議論の最中で出てきた変更の予定が TODO とか FIXME となってコメントに残されているか
    • チケットスコープ外のことは記録を残す
• この言語では/チームでは/プロダクトではしない書き方がないか
  • 例： 複数言語を扱うチームの場合、三項演算子を使わないなど（言語仕様の勘違い防止）
  • スネークケース、キャメルケース、命名規則
  • ビジネスロジックの置き場など、既存のルールに則っているか

設計面

• 選択肢を網羅できているか
  • パターンを網羅しできるだけ選択肢をあげる
    • 「ない」と思った選択肢も残す
  • 常に新たな選択肢がないか振り返る
• 非機能要件も考慮できているか
  • NGケース: 非機能要件を認識しないまま進める
  • OKケース: 求められる非機能要件の基準感を定めていく
  • 例：一覧表示したいという要件
    • ページネーションはどうするか？
    • 1ページに表示する件数は？
    • 関連情報は全て表示する？限られた要素だけ表示？
• 拡張性、工数感
• 要件や工数のmust/wantと選択肢に対する評価が正しいか
  • NGケース: must/wantで分けない
  • OKケース: ステークホルダーにmust/wantの確認が取れている