Javaのコードレビューという想定で、気をつけなければなあという観点を列挙していきます。
粒度がバラバラだったりします。また、機能、非機能要件と区別していますがプロジェクトによっては、非機能要件であっても機能側の考慮に含まれていたりしますので考えやすく、覚えやすくするための便宜上のものになります。
大前提の観点
プロジェクトでルールがあればそれを守りましょうという観点。
- コーディング規約に則っているか
- Lintツールやフォーマッターが掛かっているか
機能の観点
要件や設計の内容を過不足なく実装できているかを確認する観点。
- 権限やオブジェクトの状態、画面遷移などのパターンをすべて考慮できているか
- データ項目の過不足やデータの処理に漏れはないか
- エラーハンドリングが適切に実装できているか
非機能要件の観点
特性をどのように定義、分類分けするかの議論はありますが、「可読性」、「信頼性」、「拡張性」に分けて列挙します。
可読性
- 適切な命名か
- コメントはつけられているか
- 説明変数や要約変数が良さそうな場合に使えているか
- try-catch文内で過剰にコードを含んでいないか
- 不要な一時変数がないか
- 早期リターンになっているか
信頼性
- トランザクションが正しく実装できているか
- セキュリティ問題のあるコードになっていないか
- 不要なライブラリ、異なるライブラリを読み込んでいないか
- 間違った依存関係になっていないか
- 過剰にオブジェクト生成していないか
- ループの中で全く同じことを繰り返していないか(特に外部アクセスを伴う処理)
- 大きなものをメモリに持とうとしていないか
- 変数のスコープが過剰に大きくなっていないか
拡張性
- 同じことをしている処理は同じ実装内容になっているか≒一貫性があるか(メソッド切り出しや継承で共通化されていること、最低限ベタ書きでもよいので同じ内容になっていること)
- 共通な処理はメソッドや継承で共通化できているか
- マスタがあるものはマスタから取得できているか
- ファイルパスや外部アクセスの接続先など環境依存するものを中心に設定ファイルに切り出せているか
- 定数化するものは定数化しているか
- 適切にクラス分けされているか
- クラスの役割は適切か
- 適切にメソッド分けされているか
- メソッドの役割は適切か
- ループやif文のネストが深くなりすぎていないか(目安は3階層)
- 変数の値がやむを得ない場合を除いて書き換えられていないか