コードレビューの指針

概要

どうも、サイバーエージェントでABEMAの開発をしているシュータといいます。

表題の通り、レビュワー視点でのコードレビューの指針となるものを以下の項目に沿って端的にまとめていきます。

• コードレビューの目的
• チェック項目
• レビューする際のポイントやコツ
• 参考リンク

コードレビューのやり方ははチームやプロジェクトによって異なりますので
現場の方針に習うのが前提ですが、その上で役立つ情報があれば幸いです。

コードレビューの目的

そもそもコードレビューの目的はどういったものでしょうか？
コードレビューの目的は人によってもスタンスが異なりますが
以下の要素は大事と言えるかと思います。

• 書かれたコードが目的を達成しているかについて合意を形成する
  • 言い換えると仕様と実装が一致しているか確認することとも言えますが、仕様の方が間違っているか、実際のユースケースを網羅できていない場合もあります。
    その場合はPM等の関係者に相談し、整理しましょう。
  • このプロセスを通じてチームのエンジニアに変更内容を共有することもでき、メンテナンスできる人も増え、ひいては属人性の排除にも繋がります。
• コードの品質を高める
  • 次章のチェック項目に具体的に記載しますが、可読性やコードのスタイル・保守性・拡張性など様々な観点からコードをチェックします。
• バグが存在しないか確認する
  • コード自体に不具合がないか、変更によって既存の状態に影響を与えることがないか、将来的なバグの温床になることはないかなどをチェックできると良いです。

上記には掲げていませんが
知識の共有や教育といったことを目的とすることもあるかと思います。

ちなみにgoogleのコードレビューガイドには次のように
コードレビューの目的が記載されています。

コードレビューの主な目的は、Googleのコードベースの全体的なコードの状態が時間の経過とともに改善されていることを確認することです。

チェック観点

箇条書きベースで端的に示していきます。

• きちんと動作するコードか

• 全体的な設計や詳細の書き方に関して、ベストな選択肢を採用した記述になっているか

  • 仕様を実現する実装に関して、複数の記述パターンがある場合、その中でも最善の方法を採用できているか
  • より良い書き方はないか
  • 自分であればどのようにコードを書くか考えてみる

• コードのロジックに過不足はないか

• 変更はプロジェクトのスタンダードパターンに沿っているか

• 可読性は問題ないか

  • 変数や関数の命名はわかりやすいか
  • コード規約に沿っているか
  • マジックナンバーなどは存在しないか
  • ネストが深くなっていないか

• テストは過不足なく網羅的に書かれているか

• コメントは適切になされているか

  • 簡潔なコメントか
  • FIXMEやTODOなどどういった類のものなのか明確にされているか
  • 必要に応じて参照のリンクがあるか
  • 冗長であったり、コードから読み取れるものを書いていないか

• 既にあるコードが再実装されていないか

  • 意図を持って再実装されている場合なのか、存在を知らなかっただけなのかによって確認しましょう

• セキュリティ面での欠陥がないか

  • 疑わしい場合はエキスパートに相談しましょう

• 不要なソースコードはないか

• コミットの粒度・メッセージの内容は適切か

• DRYを意識できているか

• スペルは正しいか

ここにあげたものはかなり汎用的なものであり、
ポジションやユースケースに応じて、
チェック項目は最適化させて行くことが望ましいです。
例えばwebのフロントエンドであれば、DOMの構造やアクセシビリティ・パフォーマンスなども加えて考慮できると良いと思います。

レビューする際のポイントやコツ

• 断定的な判断は避け、不明確な場合は質問する。
  • レビュイーの考慮もれや無知を決めつけるのは良くありません。質問ベースで対話した方が良いです。
• 肯定的なフォードバックも行う
• コメントに[must][imo][nits][ask]などのラベリングを行う。
  • レビュイーにとってはどういった類のものなのか伝わりやすく、コミュニケーションの質が上がります。
• 場合に応じて口頭でのコミュニケーションも併用する
  • テキストベースだと認識を合わせずらい内容は口頭ですり合わせ、やりとりの内容をコメントに残すと良いです。
• 修正を依頼する際はなるべく根拠を示す

コードレビューで行ってはいけないこと

• 人格の否定
  • レビューすべきはコードであり、エンジニアの人格ではありません。
    否定を行うような発言をするとレビュイーの萎縮やチーム内部の関係が悪化することに繋がりかねないので気をつけましょう。

参考リンク

• Code Review Best Practices - Palantir Blog
• eng-practices
  Google's Engineering Practices documentation
• コードレビューの目的と考え方 - osa_k’s diary
• コードレビューを成功させるためにCTOが考えるべき7つのことー監修：高遠和也（株式会社LIG CTO）
  
• コードレビュー チェックリスト
• 「リーダブルコード適用 チェックリスト」を作ってみました
• 書籍『リーダブルコード ―より良いコードを書くためのシンプルで実践的なテクニック』

後書き

チェック観点などは他にも挙げられるものが多く存在すると思います。
コードレビューや良いコードとはどういったものかという書籍やwebドキュメントは数多く存在しますし、私自身自分の持ち得る情報を全て整理できていないかもしれません。
ですので今後もこの資料をアップデートできればと思いますし、
気がついたところがあれば、コメントをいただけると幸いです。