リファクタリング

概要

外部動作を変えずにコード構造と可読性を改善します。リファクタリングは漸進的な進化であり、革命ではありません。スクラッチからの書き直しではなく、既存コードの改善に使用してください。

使用する場面

以下の場合にこのスキルを使用してください:

• コードが理解または保守しにくい
• 関数/クラスが大きすぎる
• コードの匂いに対処する必要がある
• コード構造のせいで機能追加が難しい
• ユーザーが「このコードをクリーンアップして」「リファクタリングして」「改善して」と言った

---

リファクタリングの原則

黄金ルール

1. 動作は保持される - リファクタリングはコードが何をするか（の動作）を変えず、どう（の方法）を変える
2. 小さなステップ - 小さな変更を加え、その後各々テストする
3. バージョン管理は友達 - 安全な状態の前後でコミットする
4. テストは必須 - テストなしでは、リファクタリングではなく編集している
5. 一度に1つのこと - リファクタリングと機能変更を混ぜない

リファクタリングすべきでない時

- 機能し、再度変わることないコード（壊れてなければ修正するな）
- テストなしの重要な本番コード（まずテストを追加する）
- 期限が厳しい時
- 「ただそれらしいから」- 明確な目的が必要

---

よくあるコードの匂いと修正方法

1. 長いメソッド/関数

# 悪い例: すべてを実行する200行の関数
- async function processOrder(orderId) {
-   // 50行: 注文を取得
-   // 30行: 注文を検証
-   // 40行: 価格計算
-   // 30行: 在庫更新
-   // 20行: 出荷作成
-   // 30行: 通知送信
- }

# 良い例: フォーカスされた関数に分解
+ async function processOrder(orderId) {
+   const order = await fetchOrder(orderId);
+   validateOrder(order);
+   const pricing = calculatePricing(order);
+   await updateInventory(order);
+   const shipment = await createShipment(order);
+   await sendNotifications(order, pricing, shipment);
+   return { order, pricing, shipment };
+ }

2. 重複したコード

# 悪い例: 複数の場所で同じロジック
- function calculateUserDiscount(user) {
-   if (user.membership === 'gold') return user.total * 0.2;
-   if (user.membership === 'silver') return user.total * 0.1;
-   return 0;
- }
-
- function calculateOrderDiscount(order) {
-   if (order.user.membership === 'gold') return order.total * 0.2;
-   if (order.user.membership === 'silver') return order.total * 0.1;
-   return 0;
- }

# 良い例: 共通ロジックを抽出
+ function getMembershipDiscountRate(membership) {
+   const rates = { gold: 0.2, silver: 0.1 };
+   return rates[membership] || 0;
+ }
+
+ function calculateUserDiscount(user) {
+   return user.total * getMembershipDiscountRate(user.membership);
+ }
+
+ function calculateOrderDiscount(order) {
+   return order.total * getMembershipDiscountRate(order.user.membership);
+ }

3. 大きなクラス/モジュール

# 悪い例: あまりに多くを知っているゴッドオブジェクト
- class UserManager {
-   createUser() { /* ... */ }
-   updateUser() { /* ... */ }
-   deleteUser() { /* ... */ }
-   sendEmail() { /* ... */ }
-   generateReport() { /* ... */ }
-   handlePayment() { /* ... */ }
-   validateAddress() { /* ... */ }
-   // さらに50個以上のメソッド...
- }

# 良い例: クラスごとに単一責任
+ class UserService {
+   create(data) { /* ... */ }
+   update(id, data) { /* ... */ }
+   delete(id) { /* ... */ }
+ }
+
+ class EmailService {
+   send(to, subject, body) { /* ... */ }
+ }
+
+ class ReportService {
+   generate(type, params) { /* ... */ }
+ }
+
+ class PaymentService {
+   process(amount, method) { /* ... */ }
+ }

4. 長いパラメータリスト

# 悪い例: パラメータが多すぎる
- function createUser(email, password, name, age, address, city, country, phone) {
-   /* ... */
- }

# 良い例: 関連するパラメータをグループ化
+ interface UserData {
+   email: string;
+   password: string;
+   name: string;
+   age?: number;
+   address?: Address;
+   phone?: string;
+ }
+
+ function createUser(data: UserData) {
+   /* ... */
+ }

# さらに良い例: 複雑な構築にはビルダーパターンを使用
+ const user = UserBuilder
+   .email('test@example.com')
+   .password('secure123')
+   .name('Test User')
+   .address(address)
+   .build();

5. 機能の羨望

# 悪い例: 別のオブジェクトのデータを自身より多く使用するメソッド
- class Order {
-   calculateDiscount(user) {
-     if (user.membershipLevel === 'gold') {
+       return this.total * 0.2;
+     }
+     if (user.accountAge > 365) {
+       return this.total * 0.1;
+     }
+     return 0;
+   }
+ }

# 良い例: ロジックをデータを所有するオブジェクトに移動
+ class User {
+   getDiscountRate(orderTotal) {
+     if (this.membershipLevel === 'gold') return 0.2;
+     if (this.accountAge > 365) return 0.1;
+     return 0;
+   }
+ }
+
+ class Order {
+   calculateDiscount(user) {
+     return this.total * user.getDiscountRate(this.total);
+   }
+ }

6. プリミティブ型への執着

# 悪い例: ドメイン概念にプリミティブ型を使用
- function sendEmail(to, subject, body) { /* ... */ }
- sendEmail('user@example.com', 'Hello', '...');
-
- function createPhone(country, number) {
-   return `${country}-${number}`;
- }

# 良い例: ドメイン型を使用
+ class Email {
+   private constructor(public readonly value: string) {
+     if (!Email.isValid(value)) throw new Error('Invalid email');
+   }
+   static create(value: string) { return new Email(value); }
+   static isValid(email: string) { return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email); }
+ }
+
+ class PhoneNumber {
+   constructor(
+     public readonly country: string,
+     public readonly number: string
+   ) {
+     if (!PhoneNumber.isValid(country, number)) throw new Error('Invalid phone');
+   }
+   toString() { return `${this.country}-${this.number}`; }
+   static isValid(country: string, number: string) { /* ... */ }
+ }
+
+ // 使用例
+ const email = Email.create('user@example.com');
+ const phone = new PhoneNumber('1', '555-1234');

7. マジックナンバー/文字列

# 悪い例: 説明のない値
- if (user.status === 2) { /* ... */ }
- const discount = total * 0.15;
- setTimeout(callback, 86400000);

# 良い例: 名前付き定数
+ const UserStatus = {
+   ACTIVE: 1,
+   INACTIVE: 2,
+   SUSPENDED: 3
+ } as const;
+
+ const DISCOUNT_RATES = {
+   STANDARD: 0.1,
+   PREMIUM: 0.15,
+   VIP: 0.2
+ } as const;
+
+ const ONE_DAY_MS = 24 * 60 * 60 * 1000;
+
+ if (user.status === UserStatus.INACTIVE) { /* ... */ }
+ const discount = total * DISCOUNT_RATES.PREMIUM;
+ setTimeout(callback, ONE_DAY_MS);

8. ネストされた条件

# 悪い例: 矢印のようなコード
- function process(order) {
-   if (order) {
-     if (order.user) {
-       if (order.user.isActive) {
-         if (order.total > 0) {
-           return processOrder(order);
+         } else {
+           return { error: 'Invalid total' };
+         }
+       } else {
+         return { error: 'User inactive' };
+       }
+     } else {
+       return { error: 'No user' };
+     }
+   } else {
+     return { error: 'No order' };
+   }
+ }

# 良い例: ガード句/早期リターン
+ function process(order) {
+   if (!order) return { error: 'No order' };
+   if (!order.user) return { error: 'No user' };
+   if (!order.user.isActive) return { error: 'User inactive' };
+   if (order.total <= 0) return { error: 'Invalid total' };
+   return processOrder(order);
+ }

# さらに良い例: Result型を使用
+ function process(order): Result<ProcessedOrder, Error> {
+   return Result.combine([
+     validateOrderExists(order),
+     validateUserExists(order),
+     validateUserActive(order.user),
+     validateOrderTotal(order)
+   ]).flatMap(() => processOrder(order));
+ }

9. デッドコード

# 悪い例: 未使用のコードが残っている
- function oldImplementation() { /* ... */ }
- const DEPRECATED_VALUE = 5;
- import { unusedThing } from './somewhere';
- // コメントアウトされたコード
- // function oldCode() { /* ... */ }

# 良い例: 削除する
+ // 未使用の関数、インポート、コメントアウトされたコードを削除する
+ // 必要になったら、gitの履歴がある

10. 不適切な親密さ

# 悪い例: あるクラスが別のクラスに深く到達している
- class OrderProcessor {
-   process(order) {
-     order.user.profile.address.street;  // 親密すぎる
-     order.repository.connection.config;  // カプセル化を破る
+   }
+ }

# 良い例: 聞く、教えない
+ class OrderProcessor {
+   process(order) {
+     order.getShippingAddress();  // Orderは取得方法を知っている
+     order.save();  // Orderは自身の保存方法を知っている
+   }
+ }

---

メソッド抽出のリファクタリング

ビフォー・アフター

# 前: 1つの長い関数
- function printReport(users) {
-   console.log('USER REPORT');
-   console.log('============');
-   console.log('');
-   console.log(`Total users: ${users.length}`);
-   console.log('');
-   console.log('ACTIVE USERS');
-   console.log('------------');
-   const active = users.filter(u => u.isActive);
-   active.forEach(u => {
-     console.log(`- ${u.name} (${u.email})`);
-   });
-   console.log('');
-   console.log(`Active: ${active.length}`);
-   console.log('');
-   console.log('INACTIVE USERS');
-   console.log('--------------');
-   const inactive = users.filter(u => !u.isActive);
-   inactive.forEach(u => {
-     console.log(`- ${u.name} (${u.email})`);
-   });
-   console.log('');
-   console.log(`Inactive: ${inactive.length}`);
- }

# 後: メソッドを抽出
+ function printReport(users) {
+   printHeader('USER REPORT');
+   console.log(`Total users: ${users.length}\n`);
+   printUserSection('ACTIVE USERS', users.filter(u => u.isActive));
+   printUserSection('INACTIVE USERS', users.filter(u => !u.isActive));
+ }
+
+ function printHeader(title) {
+   const line = '='.repeat(title.length);
+   console.log(title);
+   console.log(line);
+   console.log('');
+ }
+
+ function printUserSection(title, users) {
+   console.log(title);
+   console.log('-'.repeat(title.length));
+   users.forEach(u => console.log(`- ${u.name} (${u.email})`));
+   console.log('');
+   console.log(`${title.split(' ')[0]}: ${users.length}`);
+   console.log('');
+ }

---

型安全性の導入

型なしから型ありへ

# 前: 型なし
- function calculateDiscount(user, total, membership, date) {
-   if (membership === 'gold' && date.getDay() === 5) {
-     return total * 0.25;
-   }
-   if (membership === 'gold') return total * 0.2;
-   return total * 0.1;
- }

# 後: 完全な型安全性
+ type Membership = 'bronze' | 'silver' | 'gold';
+
+ interface User {
+   id: string;
+   name: string;
+   membership: Membership;
+ }
+
+ interface DiscountResult {
+   original: number;
+   discount: number;
+   final: number;
+   rate: number;
+ }
+
+ function calculateDiscount(
+   user: User,
+   total: number,
+   date: Date = new Date()
+ ): DiscountResult {
+   if (total < 0) throw new Error('Total cannot be negative');
+
+   let rate = 0.1; // デフォルトブロンズ
+
+   if (user.membership === 'gold' && date.getDay() === 5) {
+     rate = 0.25; // ゴールド向け金曜ボーナス
+   } else if (user.membership === 'gold') {
+     rate = 0.2;
+   } else if (user.membership === 'silver') {
+     rate = 0.15;
+   }
+
+   const discount = total * rate;
+
+   return {
+     original: total,
+     discount,
+     final: total - discount,
+     rate
+   };
+ }

---

リファクタリングのデザインパターン

Strategy パターン

# 前: 条件付きロジック
- function calculateShipping(order, method) {
-   if (method === 'standard') {
-     return order.total > 50 ? 0 : 5.99;
-   } else if (method === 'express') {
-     return order.total > 100 ? 9.99 : 14.99;
+   } else if (method === 'overnight') {
+     return 29.99;
+   }
+ }

# 後: Strategy パターン
+ interface ShippingStrategy {
+   calculate(order: Order): number;
+ }
+
+ class StandardShipping implements ShippingStrategy {
+   calculate(order: Order) {
+     return order.total > 50 ? 0 : 5.99;
+   }
+ }
+
+ class ExpressShipping implements ShippingStrategy {
+   calculate(order: Order) {
+     return order.total > 100 ? 9.99 : 14.99;
+   }
+ }
+
+ class OvernightShipping implements ShippingStrategy {
+   calculate(order: Order) {
+     return 29.99;
+   }
+ }
+
+ function calculateShipping(order: Order, strategy: ShippingStrategy) {
+   return strategy.calculate(order);
+ }

Chain of Responsibility パターン

# 前: ネストされた検証
- function validate(user) {
-   const errors = [];
-   if (!user.email) errors.push('Email required');
+   else if (!isValidEmail(user.email)) errors.push('Invalid email');
+   if (!user.name) errors.push('Name required');
+   if (user.age < 18) errors.push('Must be 18+');
+   if (user.country === 'blocked') errors.push('Country not supported');
+   return errors;
+ }

# 後: Chain of Responsibility
+ abstract class Validator {
+   abstract validate(user: User): string | null;
+   setNext(validator: Validator): Validator {
+     this.next = validator;
+     return validator;
+   }
+   validate(user: User): string | null {
+     const error = this.doValidate(user);
+     if (error) return error;
+     return this.next?.validate(user) ?? null;
+   }
+ }
+
+ class EmailRequiredValidator extends Validator {
+   doValidate(user: User) {
+     return !user.email ? 'Email required' : null;
+   }
+ }
+
+ class EmailFormatValidator extends Validator {
+   doValidate(user: User) {
+     return user.email && !isValidEmail(user.email) ? 'Invalid email' : null;
+   }
+ }
+
+ // チェーンを構築
+ const validator = new EmailRequiredValidator()
+   .setNext(new EmailFormatValidator())
+   .setNext(new NameRequiredValidator())
+   .setNext(new AgeValidator())
+   .setNext(new CountryValidator());

---

リファクタリングステップ

安全なリファクタリングプロセス

1. 準備
   - テストが存在することを確認（必要に応じて作成）
   - 現在の状態をコミット
   - フィーチャーブランチを作成

2. 特定
   - 対処するコードの匂いを見つける
   - コードが何をするか理解する
   - リファクタリングを計画する

3. リファクタリング（小さなステップ）
   - 1つの小さな変更を加える
   - テストを実行
   - テストが成功したらコミット
   - 繰り返す

4. 検証
   - すべてのテストが成功
   - 必要に応じて手動テスト
   - パフォーマンスは変わらないか改善

5. クリーンアップ
   - コメントを更新
   - ドキュメントを更新
   - 最終コミット

---

リファクタリングチェックリスト

コード品質

• 関数は小さい（50行未満）
• 関数は1つのことを実行
• 重複したコードなし
• 説明的な名前（変数、関数、クラス）
• マジックナンバー/文字列なし
• デッドコードが削除されている

構造

• 関連コードが一緒にある
• モジュール境界が明確
• 依存関係は一方向に流れている
• 循環依存がない

型安全性

• すべてのパブリックAPIに型が定義されている
• 正当性なしのany型がない
• null許容型が明示的にマークされている

テスト

• リファクタリングされたコードはテストされている
• テストはエッジケースをカバー
• すべてのテストが成功

---

一般的なリファクタリング操作

操作	説明
メソッド抽出 (Extract Method)	コード断片をメソッドに
クラス抽出 (Extract Class)	動作を新しいクラスに移動
インターフェース抽出 (Extract Interface)	実装からインターフェースを作成
メソッドのインライン化 (Inline Method)	メソッド本体を呼び出し元に戻す
クラスのインライン化 (Inline Class)	クラスの動作を呼び出し元に移動
メソッドを上に (Pull Up Method)	メソッドをスーパークラスに移動
メソッドを下に (Push Down Method)	メソッドをサブクラスに移動
メソッド/変数の名前変更 (Rename)	明確性を改善
パラメータオブジェクトの導入	関連パラメータをグループ化
条件をポリモーフィズムで置換	switch/ifの代わりにポリモーフィズム
マジックナンバーを定数で置換	名前付き定数
条件を分解 (Decompose Conditional)	複雑な条件を分割
条件を統合 (Consolidate Conditional)	重複した条件を組み合わせ
ネストされた条件をガード句で置換	早期リターン
Null オブジェクトを導入	null チェックを排除
型コードをクラス/Enum で置換	強い型付け
継承を委譲で置換	継承より構成