yynsmk's tech blog

何でもできる=何にもできない

Googleに学ぶコードレビューで確認すべきポイント

Googleがコードレビューのガイドラインを公開してくれました。
google.github.io google.github.io

そのガイドラインを参考に、「コードレビューで確認すべきポイント」を簡単にまとめました。
コードレビューって具体的に何をすればいいか分からないって人はぜひ読んでみてください。

コードレビューとは?

そもそもコードレビューとは何か?

”コードの作者以外の人がそのコードを確認するプロセス”

何のためにするのか?

”コードとプロダクトの品質を維持するため”

コードレビューの原則

”完璧なコードはないが、より良いコードは存在する”

レビュアーは開発者に完璧さではなく、継続的な改善を求める行動を取る必要があります。
また、CL(changelistの略、GitHub等で見れるコードの変更点)が完璧でなくても、システム全体のコードの健全性を改善できることが分かれば、レビューを承認してOKです。

コードレビューで確認すべきポイント

ここから、レビュー時にどんな項目をチェックすればいいのかを見ていきます。

設計

最重要項目。
CLがシステムにとって適切な設計になっているか確認します。
具体的には、CL内のコード同士のもつ関係に意味があるか?、CLがシステムの他の部分とうまく統合できているか?などを検討します。

機能

CLが開発者の意図通りに動作するか確認します。
また、開発者の意図がエンドユーザおよび、将来そのコードを利用する人にとって適切かどうかも確認します。

多くの場合、開発者はレビュー依頼前にCLが正しく機能するかどうかをテストします。
そのため、レビュアーは以下のような点に特に注目するといいです。

  • UIへの変更等、ユーザに影響が出る部分
  • 並列プログラミングにおいてデッドロックや競合状態が起こらない安全な設計になっているか

複雑さ

コードが十分にシンプルか確認します。
もう少し具体的に言うと、コードを読む人にとって理解しやすいか、別の開発者がコードを利用したときにバグが起きにくいか等を見ます。
また、開発者が将来を見越して必要以上にコードを汎用的にしたり、不必要な機能を追加していないか(オーバーエンジニアリング)にも注意します。

テスト

CLに適した単体テストや結合テスト、E2Eテストがあり、それらが実際に有用であるか確認します。
テストのためのテストは用意せず、人が有用性を判断します。
コードに不備があったときに本当にテストは失敗するのか?有用なアサーションを出してくれるのか?等を見ます。

命名

変数やクラス、メソッドに明確な名前が付けられているか、名前を見ればその変数やメソッドが何を表しているか分かるか確認します。

コメント

必要十分なコメントがつけられており、内容が明確で分かりやすいか確認します。
コメントは複雑なアルゴリズムや正規表現につける場合を除き、コードの処理内容ではなく、コードの存在意義を説明するためにつけます。
コードの処理内容に説明が必要ならば、そのコードはもっとシンプルにできるはずです。

スタイル

コードが組織内のスタイルに則って書かれているか確認します。
また、一つのCLでスタイルの変更とその他の変更が混ざっていないかも確認します。
例えば、ある機能の追加とファイルのフォーマットという変更を1つのCLにしてしまうと、マージやロールバックの際に問題が発生しやすくなるため、CLを分ける必要があるのです。

ドキュメント

CLに関連するドキュメントにも必要な変更が加わっているか確認します。
例えば、新たなテストコードを作成したにも関わらず、その実行方法がドキュメントに記載されていない場合は開発者に追記を依頼します。


以上、簡単なまとめでした。
これを参考にどんどんレビューしていきましょう!
もっと詳しいことが知りたいって人は公式ガイドラインを見ましょう。
レビューを早くする方法やレビューコメントの書き方、CLの作り方なども説明されているので読む価値ありです。