実践的なレビュー手法

ストーリー

「さて、いよいよ実践だ。今日から実際のPRを題材にして レビュー力を鍛えていくぞ」

松本先輩がPRの一覧を開いた。

「レビューにはいくつかのアプローチがある。 どんなPRに対してどの手法が有効か、 実際のコードを見ながら体感していこう」

レビューアプローチの種類

1. トップダウンアプローチ

全体像から詳細へと進むアプローチです。大きな変更に適しています。

手順:
1. PR説明文で目的と背景を理解
2. 変更ファイル一覧で全体の構造を把握
3. アーキテクチャレベルの設計を確認
4. 各ファイルの詳細を確認
5. テストの十分性を確認

2. テストファーストアプローチ

テストコードを最初に読むことで、変更の仕様を理解してから実装を確認します。

手順:
1. テストファイルを最初に読む
   → 「この変更は何をするのか」を理解
2. テストから読み取った仕様と実装を照合
   → 「テスト通りに実装されているか」を確認
3. テストに不足がないか確認
   → 「テストされていないケースはないか」

3. 変更起点アプローチ

変更の起点となるファイルから辿るアプローチです。

// 例: APIエンドポイントの追加
// 変更起点: ルーティング定義

// 1. router.ts から読む
router.post('/api/orders', orderController.create);

// 2. controller に辿る
// 3. service に辿る
// 4. repository に辿る
// 5. テストを確認

実践：レビューの流れ

以下のPRを題材に、トップダウンアプローチでレビューしてみましょう。

PR: 「注文機能のバリデーション追加」

PR #42: 注文のバリデーションを追加
変更ファイル: 5ファイル (+180行, -20行)

src/domain/value-objects/OrderQuantity.ts  (+35)
src/domain/value-objects/OrderAmount.ts    (+40)
src/application/use-cases/CreateOrder.ts   (+45, -15)
src/adapters/in/OrderController.ts         (+25, -5)
tests/domain/value-objects/OrderQuantity.test.ts (+35)

Step 1: PR説明文の確認

## 背景
- 注文数量に0以下やマイナス値を入れてもエラーにならないバグ(#38)
- 注文金額の上限チェックがなく、異常な高額注文が発生(#39)

## 変更内容
- OrderQuantity / OrderAmount Value Object を新規作成
- CreateOrder ユースケースでバリデーション適用
- コントローラーにエラーレスポンスを追加

Step 2: ファイル構成の確認

変更の全体像:
  Value Objects (新規作成)
    ↑ 使用
  Use Case (バリデーション追加)
    ↑ 呼び出し
  Controller (エラーレスポンス追加)

✅ ドメイン駆動設計のレイヤーに沿った変更
✅ 依存の方向が正しい（外→内）
⚠️ OrderAmount のテストが見当たらない

Step 3: 各ファイルの詳細レビュー

// src/domain/value-objects/OrderQuantity.ts
export class OrderQuantity {
  private constructor(private readonly value: number) {}

  static create(value: number): OrderQuantity {
    if (!Number.isInteger(value)) {
      throw new Error('Quantity must be an integer');
    }
    if (value <= 0) {
      throw new Error('Quantity must be positive');
    }
    if (value > 100) {
      throw new Error('Quantity must not exceed 100');
    }
    return new OrderQuantity(value);
  }

  getValue(): number {
    return this.value;
  }
}

// レビューコメント:
// [Praise] Value Object パターンが適切に実装されています。
// private constructor で不正な生成を防いでいるのが良いですね。

// [Should Fix] エラーメッセージに最大値をハードコードするより、
// 定数を使い、メッセージにも含めると保守性が上がります:
// static readonly MAX_QUANTITY = 100;
// `Quantity must not exceed ${OrderQuantity.MAX_QUANTITY}`

// src/application/use-cases/CreateOrder.ts
export class CreateOrderUseCase {
  async execute(input: CreateOrderInput): Promise<Order> {
    const quantity = OrderQuantity.create(input.quantity);
    const amount = OrderAmount.create(input.unitPrice * input.quantity);

    // ... 注文作成ロジック
  }
}

// レビューコメント:
// [Must Fix] 金額計算(unitPrice * quantity)をユースケースで
// 行っていますが、これはドメインロジックです。
// Order エンティティか、専用のドメインサービスに移すことを提案します。
// 金額計算のロジックが分散すると、整合性を保つのが難しくなります。

レビュー時の思考プロセス

レビュー時に頭の中で行う確認のフローです。

各コード行に対して:

「この変更は必要か？」
  → No: 不要なコードの混入を指摘
  → Yes: 次へ

「正しく動作するか？」
  → エッジケースは？
  → エラー時の挙動は？
  → 並行処理の問題は？

「より良い方法はないか？」
  → 可読性は改善できるか？
  → パフォーマンスは問題ないか？
  → 既存のユーティリティで代替できるか？

「テストは十分か？」
  → このケースのテストはあるか？
  → 失敗するテストはないか？

レビューの時間配分

30分のレビュー時間を効果的に使う配分の目安です。

0-5分:   PR説明文の読解、全体像の把握
5-10分:  テストコードの確認（仕様の理解）
10-20分: 実装コードの確認（正確性、設計、可読性）
20-25分: セキュリティ、パフォーマンスの確認
25-30分: コメントの整理、レビューの提出

まとめ

チェックリスト

• 3つのレビューアプローチを使い分けられる
• PR全体の構造を把握してからレビューを始められる
• レビュー時の思考プロセスを意識できる
• 30分のレビュー時間を適切に配分できる
• レビューコメントを体系的にまとめて提出できる

次のステップへ

実践的なレビュー手法を学んだら、次はコードスメルの検出方法を学びます。問題のあるコードの「匂い」を嗅ぎ分ける力を身につけましょう。

推定読了時間: 30分