はじめに
今までレビュアーになったことがないのに、急にコードレビューをお願いされて、どういう点に気をつけてチェックをすればよいか分からないといったことはありませんか？
今日はそんなときにどんな観点でコードレビューをすればよいのかを解説していきます。
今までの経験値＋一般論＋実際に現場へ導入するときのTIPSを交えて解説していきます。
コードレビューで押さえておきたい４つのポイント
いきなりコードレビューをお願いされても。。
コードレビューをするときは、簡単なものから経験値が必要なものまで、いくつかポイントがあります。ひとつずつ解説していきますので一緒に見ていきましょう！
【機能性】設計書通りの仕様でコードが書かれているか
会社でコードを書くにあたって、よほどでない限りコーディングの前に設計書を書きます。設計書といっても基本設計書しかない現場や、詳細設計まできっちり揃っている現場など様々だと思いますが、ここではコーディングのインプットとなる資料のことを設計書と位置づけています。
一番わかり易い例でいくと、ウォーターフォール型の開発スタイルを取っている場合は、以下の順序で開発工程が進んでいきます。
- 要件定義
- 基本設計
- 詳細設計
- 実装・単体テスト
- 結合テスト
- 総合テスト
上から要件定義は総合テスト、基本設計は結合テスト、詳細設計は実装・単体テストというように、必ず設計工程に対応するテスト工程が用意されています。このインプットとなる設計書通りに実装がなされているかを確認することが大前提となります。
この観点については、コーディングに関して実力者でなくとも、ある程度コードが読める実力さえあれば設計書に沿っているかどうかなので、比較的難易度が低いものになります。
うちの現場アジャイルなので、この工程通りじゃないんですが。。
アジャイル開発だとしても、設計書に類する資料はあるはずなので、それは確認してみましょう
【可読性】コーディングルールに従っているか
例えば、変数の命名規則、コメントをつける単位、改行、一行に書く長さ、ネストのルールなど。。
現場ではそれぞれルールが異なるとは思うので、まずはコーディングルールを確認、あればそれに従っているかを観点とし、なければ、一般的なコーディングルールに沿っているかの確認を行いましょう。その現場で読みやすいコードかどうかが重要です。
一般的なコーディングルールはこちらが為になります。
この観点については、開発環境のプラグイン等で自動化もできるほか、上記書籍など一般的なルールは明確にあるため、比較的難易度が低いものになります。
【言語】安全な書き方になっているか
Javaだと、ループを回すときに、通常のfor文ではなく拡張for文やStreamで回しているか、try catchではなく、 try with resourceでしっかり後処理しているかなどです。
私も最新の言語仕様はまだまだ勉強しないといけません。。
言語仕様をある程度理解している必要があること、また何が安全かについては経験も必要とすることから、難易度は中くらいとなります。
【非機能】性能やセキュリティ面で問題ないか
いわゆるN+1問題や、SQL発行するときにプレースホルダーを利用、コネクションは切っているかなど。
こちらも仕様面の理解だけではなく、言語仕様や経験値も必要になるので、難易度高めです。
まとめ
それぞれ表にまとめてみると以下の通りです。
| 観点 | 難易度 |
|---|---|
| 【機能性】設計書通りの仕様でコードが書かれているか | 易 |
| 【可読性】コーディングルールに従っているか | 易 |
| 【言語】安全な書き方になっているか | 中 |
| 【非機能】性能やセキュリティ面で問題ないか | 高 |
これら全ての観点で、依頼されたコードレビューをすべてこなすのは時間もかかるし大変です。
どこまで手を抜けるか、どこを見なければいけないか、開発者側の単体テストでどこまで担保できているかなど、チーム全体で実装、単体テスト、コードレビューの見るべき責務をしっかり分けて、質の高いコードレビューを継続できるようにチームで考えていきましょう。
