はじめに
コードレビューをしていると、レビュー観点が次のような内容に寄りがちです。
- コーディング規約に準拠しているか
- 仕様・要件を満たしているか
- 明らかなバグがないか
もちろん、これらはとても重要です。
仕様を満たしていないコードや、明らかな不具合を含むコードを見逃さないことは、コードレビューの基本です。
一方で、実際のシステム開発では、コードは一度書いて終わりではありません。
リリース後も、
- 機能追加
- 仕様変更
- 障害対応
- 調査
- 運用改善
- 他メンバーによる改修
が続きます。
そのためコードレビューでは、
「今動くか」だけでなく、「今後も保守できるか」
という観点を持つことが重要だと考えています。
この記事では、社内向けに整理したコードレビュー観点をベースに、
チーム開発で意識したいレビューの考え方をまとめます。
想定読者
主に以下のようなエンジニアの方を想定しています。
- 普段コードレビューをしている方
- レビューで何を見ればよいか迷うことがある方
- 仕様確認や規約チェック以外の観点を増やしたい方
- チームのコードレビュー品質を上げたい方
- レビュアーとして設計・保守性の観点も見られるようになりたい方
特定の言語やフレームワークに依存した話ではなく、
Webアプリケーション開発や業務システム開発など、一般的なチーム開発で使える観点を中心にしています。
この記事で伝えたいこと
コードレビューは、単なる間違い探しではありません。
レビューの目的は、実装者のミスを責めることではなく、
チームとしてより良いコードにしていくことです。
そのためには、仕様や規約だけでなく、次のような観点も見る必要があります。
- 既存設計と揃っているか
- クラスやレイヤーの責務が妥当か
- 将来の変更に強いか
- 読みやすく、意図が伝わるか
- テストしやすいか
- 障害調査・運用時に困らないか
- セキュリティや性能に問題がないか
レビューの質が上がると、
実装者だけでなくレビュアー自身の設計力・保守性への感度も上がります。
レビューで大事にしたい基本姿勢
1. レビューは間違い探しではない
コードレビューの目的は、実装者のミスを責めることではありません。
チームとして、より良いコードにしていくための活動です。
レビューでは、以下を意識します。
- チーム全体の品質を上げる
- 将来の変更に強いコードにする
- 実装者・レビュアー双方の学びにする
- 暗黙知をチーム内に共有する
- 属人化を防ぐ
レビューコメントでは、単に「直してください」と伝えるのではなく、
なぜ直した方がよいのかを添えることが重要です。
例:
この処理は今後条件が増えそうなので、分岐を分けた方が保守しやすそうです。
Controller に業務判断が入っているため、Service 側に寄せた方がレイヤーの役割が明確になりそうです。
同じ判定処理が複数箇所にあるため、共通化すると変更漏れを防げそうです。
2. 好みではなく、理由でレビューする
レビューコメントは、個人の好みではなく、以下のような理由に基づいて行います。
- 保守性
- 可読性
- 一貫性
- 安全性
- テスト容易性
- 既存設計との整合性
- 障害調査のしやすさ
「自分はこの書き方が好きではない」ではなく、
チームやプロダクトにとって、なぜその方がよいのかを伝えることが大切です。
レビュー観点
1. 仕様・要件を満たしているか
まず前提として、実装が仕様・要件を満たしているかを確認します。
見るポイント:
- 要件通りの振る舞いになっているか
- 正常系だけでなく、異常系も考慮されているか
- 境界値が考慮されているか
- 想定外の入力に対して安全に動作するか
- 既存機能への影響がないか
ただし、レビューで仕様確認だけに寄りすぎると、
「今動くコード」の確認 で終わってしまいます。
仕様を満たしていることは前提として、
その実装が 今後も保守しやすいかを見ることが重要 です。
2. 既存の設計・実装パターンと揃っているか
新しく追加されたコードが、既存の設計や実装パターンと大きくズレていないかを確認します。
見るポイント:
- 既存の同種機能と似た構成になっているか
- 独自実装しすぎていないか
- 既存の共通処理・ユーティリティ・基底クラスを活用できないか
- 既存の命名・パッケージ構成・責務分担と揃っているか
- 新しい実装方針を採用する場合、その理由が明確か
同じような機能なのに、今回だけ構成が大きく違うと、
後から読む人が理解しづらくなります。
レビューでは、
この実装は既存コードの流れに自然に乗っているか
を確認します。
例:
- 既存機能では Service に業務ロジックを書いているのに、今回だけ Controller に書いていないか
- 既存では Mapper / Repository を経由しているのに、今回だけ別ルートで DB アクセスしていないか
- 同じような入力チェックが既に共通化されているのに、個別に再実装していないか
- 既存で使っている例外クラスやエラーコード体系から外れていないか
3. デザインパターン・設計パターンを適用できないか
独自実装や複雑な分岐が増えている場合、既存の設計パターンやデザインパターンを適用することで整理できないかを確認します。
見るポイント:
- 条件分岐が増え続けそうな構造になっていないか
- 生成処理が複雑になっていないか
- 同じような前処理・後処理が複数箇所に散らばっていないか
- 将来の追加・変更時に修正箇所が増えすぎないか
- パターン適用によって、かえって複雑になっていないか
例:
-
条件ごとに処理が分かれる場合
→ Strategy パターンで処理を分離できないか -
オブジェクト生成処理が複雑な場合
→ Factory パターンで生成責務を切り出せないか -
共通の処理手順があり、一部だけ処理が異なる場合
→ Template Method 的に整理できないか -
状態によって振る舞いが大きく変わる場合
→ State パターンで整理できないか
ただし、デザインパターンは無理に使うものではありません。
大事なのは、パターン名を使うことではなく、
複雑さを整理し、変更しやすい構造にすることです。
4. クラス設計・レイヤーの責務が妥当か
クラスやメソッドの役割が適切かを確認します。
見るポイント:
- 1つのクラスが多くの責務を持ちすぎていないか
- メソッドが長くなりすぎていないか
- 業務ロジック・DBアクセス・画面/API制御が混ざっていないか
- レイヤードアーキテクチャの役割を越えていないか
- DTO / Entity / Domain / Model などの役割が曖昧になっていないか
例:
- Controller に業務判断が入りすぎていないか
- Service が画面都合やリクエスト形式に寄りすぎていないか
- Repository / Mapper に業務ロジックが入り込んでいないか
- Entity や DTO に本来持つべきでない責務が入っていないか
- 共通クラスが便利クラス化し、責務が肥大化していないか
レビューでは、
この処理は本当にこのクラス・このレイヤーにあるべきか
を確認します。
動いているから OK ではなく、置き場所が妥当かを見ることが重要です。
5. 変更に強いコードになっているか
今後の仕様変更や機能追加が発生したときに、修正しやすい構造になっているかを確認します。
見るポイント:
- 変更箇所が局所化されているか
- 同じ処理が複数箇所に重複していないか
- 条件分岐が増え続ける構造になっていないか
- 固定値・文言・コード値が直書きされていないか
- 仕様変更時に修正漏れが起きやすい構造になっていないか
例:
- 同じ判定条件が複数の Service に書かれている
- 同じエラーメッセージが複数箇所に直書きされている
- 区分値の追加があるたびに多数の if / switch を修正する必要がある
- 本来設定化すべき値がコード内にハードコードされている
レビューでは、
次に変更する人が困らないか
という視点を持ちます。
6. 読みやすく、意図が伝わるコードになっているか
コードは、書く時間よりも読まれる時間の方が長いです。
そのため、可読性は重要な品質です。
見るポイント:
- クラス名・メソッド名・変数名から意図が分かるか
- 業務用語とコード上の名称が揃っているか
- 処理の流れが自然に読めるか
- 不要に複雑な書き方をしていないか
- コメントが必要な箇所に適切に書かれているか
コメントは、「何をしているか」よりも、
なぜそうしているかを書くと価値が高いです。
良くないコメント例:
ユーザー情報を取得する
リストをループする
フラグを true にする
価値があるコメント例:
外部システム側の仕様により、退会済みユーザーも検索対象に含める
既存データとの互換性維持のため、この値のみ旧形式を許容する
バッチ再実行時の重複登録を防ぐため、登録前に存在確認を行う
7. テストしやすい構造になっているか
テストしづらいコードは、変更にも弱くなりやすいです。
テストコードの有無だけでなく、テストしやすい設計になっているかを確認します。
見るポイント:
- 単体テストで確認しやすい粒度になっているか
- 外部依存が強すぎないか
- private メソッドに重要なロジックが閉じすぎていないか
- 時刻・乱数・外部APIなどがテストしやすい形で扱われているか
- 正常系だけでなく、異常系・境界値も確認されているか
例:
- 現在時刻を直接取得しており、テストで制御できない
- 外部API呼び出しがロジック内に直接埋め込まれている
- 重要な判定ロジックが巨大な private メソッドに閉じている
- テストが正常系のみで、異常系や境界値がない
レビューでは、
テストがあるかだけでなく、
テストしやすい設計かを見ます。
8. 障害調査・運用時に困らないか
本番リリース後に問題が起きたとき、調査しやすいコードになっているかを確認します。
見るポイント:
- 必要なログが出力されているか
- ログに調査に必要な情報が含まれているか
- 例外を握りつぶしていないか
- エラーメッセージが原因調査に役立つ内容になっているか
- リトライ・タイムアウト・排他制御など、運用上の考慮が必要ないか
例:
- catch した例外をログ出力せずに握りつぶしている
- エラーログに原因特定に必要な ID や処理対象が出ていない
- 外部API呼び出しにタイムアウトが設定されていない
- リトライ時に二重登録が起きる可能性がある
- バッチ処理で、どこまで処理が進んだか追えない
保守運用では、
問題が起きないことだけでなく、
問題が起きたときに調査できることも重要です。
9. セキュリティ上の問題がないか
レビューでは、セキュリティ観点も意識します。
見るポイント:
- 入力値検証が適切に行われているか
- 認証・認可のチェック漏れがないか
- SQLインジェクションやXSSにつながる実装がないか
- 個人情報や機密情報をログに出力していないか
- ファイルアップロード・ダウンロード処理に危険な実装がないか
- 外部API連携時に秘密情報を安全に扱っているか
例:
- 権限チェックなしで他ユーザーの情報を参照できる
- ユーザー入力値をそのままSQL文字列に連結している
- HTML出力時にエスケープされていない
- パスワード、トークン、個人情報をログに出している
- アップロードファイルの拡張子・サイズ・保存先の検証が不十分
セキュリティは後からまとめて見るのではなく、
日々のレビューの中で少しずつ確認することが重要です。
10. 性能・データ量の増加に耐えられるか
小さなデータでは問題なく動いても、本番データ量では問題になる実装があります。
見るポイント:
- ループ内でSQLや外部APIを何度も呼んでいないか
- 大量データを一度にメモリへ読み込んでいないか
- 不要なDBアクセスが発生していないか
- 適切なインデックスが使われる検索条件になっているか
- ページングや分割処理が必要ないか
例:
- 一覧取得後、1件ずつ詳細取得SQLを実行している
- 全件取得してからアプリ側で絞り込んでいる
- バッチ処理で大量データを一括でメモリに保持している
- 検索条件にインデックスが効きづらい書き方をしている
- 外部APIをループ内で大量に呼んでいる
レビューでは、
本番データ量でも問題ないか
という視点を持ちます。
11. トランザクション・排他制御が妥当か
更新処理では、途中失敗や同時実行を考慮する必要があります。
見るポイント:
- トランザクションの範囲が適切か
- 途中で失敗した場合に不整合が起きないか
- 同時実行時に二重登録や上書きが起きないか
- リトライ時に冪等性が保たれるか
- 外部API連携とDB更新の整合性が考慮されているか
例:
- 複数テーブル更新の途中で失敗した場合に、一部だけ更新される
- 同時に同じ申込処理が走った場合に二重登録される
- リトライによって同じ通知が複数回送信される
- DB更新後に外部API連携が失敗した場合の扱いが決まっていない
レビューでは、
失敗したとき・同時に動いたときにどうなるか
を確認します。
12. ドキュメント・補足情報が必要ないか
コードだけでは意図が伝わりにくい変更の場合、ドキュメントや補足情報が必要になります。
見るポイント:
- README や設計書の更新が必要ないか
- API仕様書の更新が必要ないか
- 環境変数・設定値の追加が共有されているか
- 運用手順やリリース手順への影響がないか
- 複雑な設計判断の理由が残されているか
例:
- 新しい環境変数を追加したが、設定手順が残っていない
- APIレスポンス項目を変更したが、仕様書が更新されていない
- バッチの実行条件を変えたが、運用手順に反映されていない
- 複雑な暫定対応を入れたが、理由や解消方針が残っていない
レビューでは、
コード以外に更新すべきものがないか
も確認します。
レビューコメントの書き方
コメントの重要度・意図を明示する
レビューコメントでは、指摘内容だけでなく、
どの程度対応が必要なのか
どういう意図のコメントなのか
が分かるようにすると、実装者が判断しやすくなります。
そのため、コメントの先頭に以下のようなラベルを付けるとよいです。
| ラベル | 意味 |
|---|---|
| [MUST] | 必ず対応してほしい指摘。バグ、仕様不備、セキュリティ問題、設計上大きな問題など |
| [WANT] | できれば対応してほしい指摘。保守性・可読性・設計改善につながるもの |
| [IMO] | 個人的な意見・提案。対応必須ではないが、検討してほしいもの |
| [NITS] | 細かい指摘。typo、命名、軽微な表現修正など |
| [ASK] | 確認・質問。実装意図や仕様理解を確認したいもの |
これにより、実装者は
必ず直すべきものなのか、提案なのか、単なる確認なのか
を判断しやすくなります。
良いレビューコメント
良いレビューコメントは、以下を満たしています。
- 指摘理由が分かる
- 代替案がある
- 必須対応なのか、提案なのかが分かる
- 個人攻撃になっていない
- チームの品質向上につながる
例:
[WANT]
この条件分岐は今後も種類が増えそうなので、Strategy 的に処理を分けると保守しやすくなりそうです。
[MUST]
Controller に業務判断が入っているため、Service 側に寄せたいです。
このままだとレイヤーの責務が曖昧になり、同種処理との一貫性も崩れそうです。
[WANT]
この値は他の箇所でも使われる可能性があるため、定数化しておくと変更漏れを防げそうです。
[MUST]
このログだと障害発生時に対象データを特定しづらいため、userId や requestId など調査に必要な情報を含めたいです。
[IMO]
この判定処理はメソッドに切り出すと、呼び出し元の処理の流れが読みやすくなりそうです。
対応必須ではないですが、可読性の観点で検討してみてください。
[ASK]
この分岐は、退会済みユーザーも対象に含める意図で合っていますか?
仕様上必要な考慮であれば、コメントかテストケースで意図を残しておくとよさそうです。
[NITS]
typo の修正です。
「adress」ではなく「address」が正しそうです。
避けたいレビューコメント
以下のようなコメントは、意図が伝わりづらく、実装者も改善しづらいです。
ここ微妙です。
この書き方は好きではないです。
なんでこうしたんですか?
普通はこう書きます。
改善する場合は、以下のようにラベルと理由を添えます。
[WANT]
この書き方だと条件が増えたときに修正箇所が増えそうなので、処理を分けた方が保守しやすいと思います。
[MUST]
既存の同種機能では Service 側に業務ロジックを寄せているため、今回も同じ構成に揃えたいです。
このままだとレイヤーの責務が崩れ、今後の修正時に影響範囲が分かりづらくなりそうです。
[ASK]
この実装方針にした理由を確認したいです。
既存の共通処理を使わず個別実装しているように見えたため、意図があれば教えてください。
[IMO]
既存の命名と揃えるなら `userStatus` より `accountStatus` の方が近いかもしれません。
ただ、業務用語として `userStatus` の方が自然であればこのままでもよいと思います。
ラベルを使うときの注意点
ラベルは便利ですが、付ければよいというものではありません。
特に以下に注意します。
- [MUST] を多用しすぎない
- [IMO] なのに強制のような書き方をしない
- [NITS] だけでレビューを埋め尽くさない
- [ASK] のつもりで詰問口調にしない
- ラベルだけでなく、理由も添える
レビューコメントでは、
重要度を明確にすることと、
相手が納得して直せる理由を伝えること
の両方が大事です。
チームで使うときのおすすめ
このような観点を一度にすべて完璧に見るのは大変です。
特にレビューに慣れていないうちは、まず以下のように段階的に使うとよいと思います。
まず意識したい5つ
- 既存の設計・実装パターンと揃っているか
- クラス設計・レイヤーの責務が妥当か
- 変更に強いコードになっているか
- 読みやすく、意図が伝わるコードになっているか
- 障害調査・運用時に困らないか
この5つを意識するだけでも、
レビューが「仕様通り動くか」の確認から、
「保守しやすいコードか」の確認に近づきます。
チーム内で観点を共有する
レビュー観点は、個人の経験に依存しやすいものです。
だからこそ、チーム内で
- どういう観点で見るのか
- どのレベルを [MUST] とするのか
- 何を [IMO] や [WANT] とするのか
- プロジェクト固有の注意点は何か
を共有しておくと、レビューのばらつきが減ります。
レビュー観点を言語化しておくことで、
レビュアーごとの暗黙知をチームの資産にしやすくなります。
まとめ
コードレビューは、
今動くコードを確認するだけでなく、未来の開発者が困らないコードにするための活動です。
レビューの質が上がると、
個人の実装力だけでなく、チーム全体の設計力・保守力も上がっていきます。
最初からすべての観点を完璧に見る必要はありません。
まずは普段のレビューで、
「このコードは次に変更する人が困らないか?」
という視点を1つ足してみるだけでも、レビューの質は変わると思います。
おわりに
この記事の内容は、もともと社内向けにコードレビュー観点を整理したものがベースです。
社内で共有したところ、
「レビューで見るべきポイントが整理されていて分かりやすい」
「レビュアーごとの差を減らすのに使えそう」
という反応がありました。
コードレビューのやり方は、チームやプロダクトの状況によって変わります。
そのため、この記事の内容が唯一の正解というわけではありません。
ただ、レビュー観点を言語化しておくことは、
チームの品質を上げるうえでかなり効果があると感じています。
この記事が、
「レビューで何を見ればよいか」
「どうコメントすれば伝わりやすいか」
を考えるきっかけになればうれしいです。