コードレビューの観点

コードレビュー観点一覧

🔧 実装レベルの観点

1. 命名が意図を適切に表現しているか

   • 関数や変数名の名前が明確になっているか確認すること

2. 早期リターンが活用されているか

   • 早期リターンをすれば else 句を削減できないか検討すること

3. コメントでしか理解できないコードになっていないか

   • 長分のコメントを書かないと伝わらないような if などになっていないか確認すること
   • 上記の状態になっていたら if 文を分解したり、別メソッドなどに分離できないか検討すること

4. Null チェックが漏れていないか

   • 関数の入力において null チェックが行われているか

5. 関数の戻り値に null を利用していないか

   • 処理の結果に null を戻すような実装をしていないか

6. 処理中においても null をもちいた判定処理を実装していなか

   • 以下の例外を除いて null をつかったロジックを実装していないか
     • 入力チェックで null を確認する
     • 利用しているライブラリが null を返却する設計になっている
   • 処理の結果として"未定"や"不明"、"結果無し"を表現したいのであれば、専用の返答用クラス(推奨)を検討するか Optional などを利用すること

7. 副作用のある処理が実装されていないか

   • void 関数などで、引数で渡したオブジェクトの状態を変更するような処理を実装していないか確認すること
   • そのような実装をしている場合は、引数のオブジェクトをクローンした上で状態を変更し返却するような実装に修正すること

8. 定数やリテラルが適切に抽出されているか

   • 定数や定義句などがコード内にべた書きになっていないか確認すること
   • そのような値はクラスの上部に static フィールドなどで定義し明示すること
   • また明示する時はわかりやすい変数名をつけること
   • 必要であれば環境変数やフレームワークの仕組みを使って外部定義化すること

9. ループや条件分岐がシンプルかつ読みやすいか

   • 不必要な if 文、必要以上に複雑な if 文が実装されていないか確認すること
     • 2 以上の条件を満たすような条件式のときは、ド・モルガンの法則をもちいると完結に実装できないか検討すること
     • 2 つ以上のネストで表現されている if 文は 1 つの if 文で表現できないか検討すること
   • 不必要な for 文、特に必要以上にネストするような for 文を実装していないか確認すること
     • 業務ロジックを実装するシーンにおいて、コレクションオブジェクトを正しく導入していれば一箇所にネストした for 文を実装することは殆どない。仮にネストするような for 文の実装に迫られた場合は、なにか見落としているモデル(コレクションオブジェクト)がないか検討すること
     • 可能な限り拡張 for 文を利用すること

10. 例外処理が適切かつ網羅的に行われているか

    • キャッチ句でキャッチして何もしないような実装になっていないか確認すること
      • 仮にロジック上発生しない検査例外が、利用しているライブラリの求めるところによりキャッチしなければならないような状況においても、キャッチした例外を非検査例外にするなどしてスローすること

11. 冗長な処理や重複コードが存在しないか

    • 業務ロジックにおいて
      • 同じような処理が複数見受けられる場合は type オブジェクト(単位、時間、金額、率、時間、日時、年、月、日 ...etc)に見落としがないか検討すること
    • 業務ロジック外において
      • まずそのコードが業務ロジックではないか確認すること（業務ロジックと見受けられる場合は domain パッケージ配下に移動できないか検討すること）
      • 接続処理など、汎用的な処理を Abstract クラスなどに分けて、それを継承する形で実装できないか検討すること

🧩 クラス設計・責務の観点

1. クラスの責務が単一であるか（SRP）

   • ドメインモデルクラス(エンティティ、値オブジェクト、ドメインサービス)にしろ、それ以外のクラス(コントローラー、DB や外部 API との接続部品)にしろ、1 つのクラスが対象としている責務が複数にまたがっていないか
   • 複数にまたがってしまっている場合は分割を検討すること

2. メソッドの粒度が適切か

   • 1 つのメソッドの長さが長くなっていないか確認すること
   • 可能であれば 1 つのメソッドの行数は 10 行以内とし、それ以上必要な時は見落としているモデルがないか検討すること

3. ドメインオブジェクトの不変性が守られているか

   • 値オブジェクトにおいてはイミュータブルオブジェクト(不変オブジェクト)になっているか確認すること
   • getter,setter が実装されていないこと、とくに lombok を利用している時は@data``@setter``@getterなどが付与されていないか確認すること

4. ドメインモデルにビジネスロジックが適切に集約されているか

   • まずドメインモデルの整理が不自然になっていないか確認すること
   • その上で、コードのロジックがどのドメインモデルに属するか考え、適切な場所に実装されているか確認すること
   • domain パッケージ配下以外の処理に業務ロジックが散在していないか確認すること
   • 外部からの入力(コントローラークラス)され DB などとやりとりし、最終的に結果を返す一連を通しで確認して不自然でないか確認すること

5. エンティティと値オブジェクトが正しく使い分けられているか

   • 一意性を担保するための識別子(ID など)を持ち、状態の変更ができるのがエンティティ
   • イミュータブルオブジェクトなため不変で値の等価性を評価するときに使えるのが値オブジェクト
     • type オブジェクトは、その特性から基本的に値オブジェクトに属する
     • ただし、値オブジェクトだからといって全て type オブジェクトではない
   • 上記の区分が曖昧になっていないか確認すること

6. 集約（Aggregate）の境界が明確かつ一貫性が保たれているか

   • ルートエンティティが正しく認識され実装されているか
     • ルートエンティティとは集約を扱う単位
       • 注文で言えば注文モデル
         • 注文という 1 つの集約の単位で様々な制約がある
         • 注文の配下には注文明細があり、注文明細への変更は必ずルートエンティティである注文モデルを経由する必要がある

7. リポジトリがドメインの視点から適切な粒度になっているか

   • リポジトリの整理や関数の粒度がドメインの視点主導で実装されているか
     • SQL 側の都合で関数名などが命名されていないか

8. ドメインサービスとユースケース(アプリケーションサービス)の責務分離が明確か

   • ドメインサービスはドメインモデル間のルールを実装するクラス
     • ドメインモデルのみでは表現仕切れない処理のみを実装しているか
     • 無理にドメインサービスを作らなければならないわけではない
   • ユースケース(アプリケーションサービス)は処理のフロー制御中心
     • ビジネスロジックを実装していないか
     • 入力と出力にドメインモデルを使っていないか
       • 入力と出力にドメインモデルを使っていると、入力元、出力先でドメインモデルをつかったロジックを書き始めてしまう

9. ユースケース単位で処理の流れが整理されているか

   • 予め設計した業務の単位と一致するようにユースケースが定義されているか
   • 一目でその業務自体の処理の流れが分かるような実装になｔ

🏛 アーキテクチャ・構造の観点

1. レイヤー（ドメイン層、アプリケーション層、インフラ層など）が明確に分離されているか

   • IF 等の仕組みを用いて明確に各レイヤーが分離されているか
   • 特にテスト容易性を意識した実装になっているか
   • 各レイヤーを設計する際には、必ずドメイン層を中心に設計されているか
   • ドメイン層にインフラ層の関心事が漏れ出していないか

2. ドメイン知識がアプリケーション外に漏れていないか

   • 業務ロジックがドメイン層以外の場所(特にコントローラー層)などに流出していないか

3. トランザクション制御が適切な層にのみ存在しているか

   • トランザクションの範囲、分離度、伝播が適切に設定されているか

4. DTO や ViewModel など、入出力モデルの分離ができているか

   • リクエストからコントローラー、コントローラーからユースケース、インフラ層から DB や外部 API など入出力にドメインモデルを利用していないか