リファクタリングの提案

ストーリー

「コードスメルを見つけるのは半分の仕事だ。残りの半分は、具体的な改善案を提案することだ」

松本先輩がホワイトボードに図を描きながら説明した。

「リファクタリングとは、外部の振る舞いを変えずに内部の構造を改善すること。レビューで提案する際は『なぜ改善すべきか』と『どう改善するか』の両方を伝えることが重要だ」

リファクタリングとは

リファクタリングとは、ソフトウェアの外部的な振る舞い（機能）を変えることなく、内部の構造を改善するプロセスです。

リファクタリングの原則:
├── 外部の振る舞い（テスト結果）は変わらない
├── 内部の構造（可読性・保守性）が改善される
├── 小さなステップで段階的に行う
└── テストがあることが前提

代表的なリファクタリング手法

1. 関数の抽出（Extract Function）

長い関数から意味のあるまとまりを別関数に切り出します。

typescript

// Before
async function registerUser(input: RegisterInput) {
  // メールアドレスのバリデーション
  const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
  if (!emailRegex.test(input.email)) {
    throw new ValidationError('Invalid email format');
  }
  const existingUser = await userRepo.findByEmail(input.email);
  if (existingUser) {
    throw new ConflictError('Email already registered');
  }

  // パスワードの強度チェック
  if (input.password.length < 8) {
    throw new ValidationError('Password must be at least 8 characters');
  }
  if (!/[A-Z]/.test(input.password)) {
    throw new ValidationError('Password must contain uppercase');
  }
  if (!/[0-9]/.test(input.password)) {
    throw new ValidationError('Password must contain a number');
  }

  const hashedPassword = await bcrypt.hash(input.password, 10);
  const user = await userRepo.save({ email: input.email, password: hashedPassword });
  await emailService.sendWelcome(user.email);
  return user;
}

// After: 関数の抽出を適用
async function registerUser(input: RegisterInput) {
  await validateEmail(input.email);
  validatePasswordStrength(input.password);

  const hashedPassword = await bcrypt.hash(input.password, 10);
  const user = await userRepo.save({ email: input.email, password: hashedPassword });
  await emailService.sendWelcome(user.email);
  return user;
}

async function validateEmail(email: string): Promise<void> {
  const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
  if (!emailRegex.test(email)) {
    throw new ValidationError('Invalid email format');
  }
  const existingUser = await userRepo.findByEmail(email);
  if (existingUser) {
    throw new ConflictError('Email already registered');
  }
}

function validatePasswordStrength(password: string): void {
  if (password.length < 8) {
    throw new ValidationError('Password must be at least 8 characters');
  }
  if (!/[A-Z]/.test(password)) {
    throw new ValidationError('Password must contain uppercase');
  }
  if (!/[0-9]/.test(password)) {
    throw new ValidationError('Password must contain a number');
  }
}

2. 早期リターン（Guard Clauses）

ネストを浅くして可読性を改善します。

typescript

// Before: 深いネスト
function processPayment(order: Order, user: User): PaymentResult {
  if (order) {
    if (order.status === 'pending') {
      if (user) {
        if (user.balance >= order.total) {
          // ここに本来の処理が書かれる
          user.balance -= order.total;
          order.status = 'paid';
          return { success: true };
        } else {
          return { success: false, error: 'Insufficient balance' };
        }
      } else {
        return { success: false, error: 'User not found' };
      }
    } else {
      return { success: false, error: 'Order is not pending' };
    }
  } else {
    return { success: false, error: 'Order not found' };
  }
}

// After: 早期リターンで平坦化
function processPayment(order: Order | null, user: User | null): PaymentResult {
  if (!order) {
    return { success: false, error: 'Order not found' };
  }
  if (order.status !== 'pending') {
    return { success: false, error: 'Order is not pending' };
  }
  if (!user) {
    return { success: false, error: 'User not found' };
  }
  if (user.balance < order.total) {
    return { success: false, error: 'Insufficient balance' };
  }

  user.balance -= order.total;
  order.status = 'paid';
  return { success: true };
}

3. 条件式のポリモーフィズム（Replace Conditional with Polymorphism）

typescript

// Before: 長い switch/if-else チェーン
function calculateShippingCost(type: string, weight: number): number {
  switch (type) {
    case 'standard':
      return weight * 100;
    case 'express':
      return weight * 200 + 500;
    case 'overnight':
      return weight * 300 + 1000;
    case 'international':
      return weight * 500 + 2000;
    default:
      throw new Error(`Unknown shipping type: ${type}`);
  }
}

// After: ストラテジーパターンで置き換え
interface ShippingStrategy {
  calculate(weight: number): number;
}

class StandardShipping implements ShippingStrategy {
  calculate(weight: number): number {
    return weight * 100;
  }
}

class ExpressShipping implements ShippingStrategy {
  calculate(weight: number): number {
    return weight * 200 + 500;
  }
}

const shippingStrategies: Record<string, ShippingStrategy> = {
  standard: new StandardShipping(),
  express: new ExpressShipping(),
  // ... 新しい配送タイプの追加が容易
};

function calculateShippingCost(type: string, weight: number): number {
  const strategy = shippingStrategies[type];
  if (!strategy) throw new Error(`Unknown shipping type: ${type}`);
  return strategy.calculate(weight);
}

4. パラメータオブジェクトの導入

typescript

// Before: パラメータが多い
function searchProducts(
  keyword: string,
  category: string,
  minPrice: number,
  maxPrice: number,
  sortBy: string,
  sortOrder: 'asc' | 'desc',
  page: number,
  limit: number,
) { /* ... */ }

// After: パラメータオブジェクトに集約
interface ProductSearchCriteria {
  keyword?: string;
  category?: string;
  priceRange?: { min: number; max: number };
  sort?: { field: string; order: 'asc' | 'desc' };
  pagination: { page: number; limit: number };
}

function searchProducts(criteria: ProductSearchCriteria) { /* ... */ }

レビューでのリファクタリング提案のコツ

提案テンプレート

[Should Fix] リファクタリング提案

現状の問題:
  この関数は〇〇と△△の2つの責務を持っており、
  変更時の影響範囲が大きくなっています。

提案:
  関数の抽出（Extract Function）を適用し、
  〇〇のロジックを別関数に切り出すことを提案します。

期待される効果:
  - テストが書きやすくなる
  - 変更の影響範囲が限定される
  - 関数名から処理の意図が読み取れる

コード例:
  (具体的なリファクタリング後のコードを記載)

リファクタリングのタイミング

レビューで提案すべきタイミング:
├── 新しいコードにスメルがある場合 → PRで修正を依頼
├── 既存コードにスメルがある場合 → 別PRでの対応を提案
└── 大規模なリファクタリングが必要 → Issue を作成して計画

⚠️ 注意: レビュー中に「ついでにリファクタリングも」は避ける
   → PRのスコープが膨らみ、レビューが困難になる

まとめ

リファクタリング手法	適用シーン	効果
関数の抽出	長い関数	可読性・テスタビリティ向上
早期リターン	深いネスト	可読性向上
ポリモーフィズム	長い条件分岐	拡張性向上
パラメータオブジェクト	多すぎる引数	インターフェースの改善

チェックリスト

リファクタリングの定義（外部振る舞いを変えず内部を改善）を説明できる
4つ以上のリファクタリング手法を適用できる
レビューで具体的なリファクタリング提案ができる
リファクタリングのタイミングを判断できる

次のステップへ

リファクタリングの提案方法を学んだら、次はペアレビューとモブレビューについて学びます。1人でのレビューだけでなく、チームでのレビュー手法を身につけましょう。

推定読了時間: 30分