コードリファクタリング手法 実践的なガイド

· · Tech stack decisions
要約

コードリファクタリング手法を使えば、新しい機能を追加せずにコードベースをきれいにできます。このガイドでは実践で重要なパターンをカバーします：メソッド抽出、メソッド合成、Branch by Abstraction、条件分岐の単純化。また、リファクタリングすべき時期と、コードをそのままにすべき時期の判断基準を学べます。

複数モニターでコードリファクタリングを行う開発者

コードリファクタリング手法：実践的なガイド

ほとんどのプログラマーはリファクタリングが大事だと知っている。ただ問題は「まず何から手をつけるのか」「どこまでやるのか」「コードの整理が必要なのか、全面書き換えが必要なのか」という判断だ。6ヶ月間、レガシー推奨システムと向き合った経験と、次はどうするかという話をここにまとめた。

Dark home office with multiple monitors showing refactoring workflow

リファクタリングが本当に必要な信号

すべての「見た目が悪い」関数がリファクタリングの対象ではない。本当に取り組む価値のある信号は、いくつかに限られている。

**複数の場所に重複が現れている。**同じデータ変換が3つの異なるコントローラーに書かれている場合、これはスタイルの問題ではない。保守の悪夢だ。1つの上流の変更が3ヶ所の修正を必要とし、必ず1ヶ所は忘れる。

**1つの変更が無関係な機能を壊す。**決済フローのバグを直すのに、ユーザープロフィールの表示が影響を受けるなら、結合度が高すぎる。別々の責務が1つのものとして扱われている。

**新しいエンジニアが30分以内に方向性を掴めない。**これは過小評価されている信号だ。ファイルの全体を読まずに何をしているか説明できないなら、それはおそらく多くのことをやり過ぎている。Appierではオンボーディングのウォークスルーをタイムしていた。15分以上かかるファイルはリファクタリングバックログに追加される。

コードのチャーン集中。 git log --stat を実行して、最もよく触られるファイルを見る。同じ3つのファイルがコミットの60%を占めているなら、それは信号だ。複雑さが高く、かつ頻繁に変更されている箇所が、まず対策すべき80/20のホットスポットだ。

リファクタリングをスキップしていい場合：コードが動作し、誰も触らず、拡張の理由がない。ボーイスカウト・ルール（きれいなコードで出よ）は良い原則だが、使ってないキャンプ場を片付ける必要はない。

メソッド抽出：80%の時間で使う手法

メソッド抽出は、長い関数から意味のあるロジックの塊を取り出して、独立した関数にすることだ。その関数の名前がドキュメントになる。

Before:

def process_order(order):
    # validate
    if not order.get('user_id'):
        raise ValueError('Missing user_id')
    if order.get('amount', 0) <= 0:
        raise ValueError('Invalid amount')
    # apply discount
    if order.get('coupon') == 'LAUNCH20':
        order['amount'] = order['amount'] * 0.8
    # save
    db.save(order)

After:

def process_order(order):
    validate_order(order)
    apply_discount(order)
    db.save(order)

def validate_order(order):
    if not order.get('user_id'):
        raise ValueError('Missing user_id')
    if order.get('amount', 0) <= 0:
        raise ValueError('Invalid amount')

def apply_discount(order):
    if order.get('coupon') == 'LAUNCH20':
        order['amount'] = order['amount'] * 0.8

「After」のバージョンは長い。それでいい。各関数には1つの理由だけで存在することになり、割引ロジックが変わるとき（そして変わる）、どこを直すか正確に分かる。

効果があるケース：元の関数が20～25行を超えている、または # validate のようなコメントを書き始めるとき。そのコメントは、まだ抽出されていない関数名だ。# apply discount と書いているなら、すでに関数を命名している。あとは抽出するだけだ。

実践的なルール：次の5行を説明するインラインコメントを追加したくなったら、それは抽出の候補だ。

メソッド合成：メソッド抽出では足りないとき

メソッド合成はもっと先に行く。1つのメソッドを抽出するのではなく、クラスやモジュール全体を、より小さく集中した部品に分解する。これが必要なのは、ファイルが600行に成長して5つの無関係な目的を果たしているとき。

パターンは：責務をはっきり分ける、各責務に自分のクラスやモジュールを与える、高いレベルで結合する。Martin Fowlerはこれを「肥大化したクラスを協力し合う小さなクラスの集団に変える」と説明している。

実際の難しさは、機械的な部分ではない。1つの責務がどこで終わり、別の責務がどこから始まるか見つけることだ。使える発想法：関数やクラスが何をするか「と」を使わずに説明できたら、おそらく1つの責務だ。「と」が必要なら分ける。

もう1つの見つけ方：ファイルの上部のインポートを見る。1つのモジュールが、データベース層、メール、ロギング、決済クライアント、レポーティングという6つの無関係なドメインからインポートしているなら、やりすぎている。インポートのクラスタごとが、その存在を主張する独立したモジュール候補だ。

メソッド合成はテスト性も高める。400行のクラスはユニットテストが難しい。4つの100行のクラスで明確なインターフェースなら、モックと分離がずっと簡単だ。

Developer at whiteboard sketching refactoring architecture

Branch by Abstraction：本番環境で運用しながらのリファクタリング

これはもっとアーキテクチャレベルだが、大規模なリファクタリングを本番環境で生き残らせる手法だ。

考え方：古い実装から新しい実装への大きな切り替えではなく、抽象化層（通常はインターフェースやプロトコル）を導入して、古い実装と新しい実装の両方がそれを満たす。新しい実装をフィーチャーフラグの後ろに置いて、両方を並行で走らせ、新しいものが安定したら古いものを削除できる。

サイドプロジェクトに関連する理由：決済統合やORM移行を再構築するとき、Branch by Abstractionなら、本番で新しいバージョンを焼きながら、古いバージョンは動き続ける。「3週間メインから分岐したままのリファクタリング・ブランチ」はない。

コスト：クリーンなインターフェースを定義するのに、前もって時間がかかる。やる価値がある時：置き換えるものがリスキー（認証、課金、データアクセス）で、フルダウンタイムの窓が取れない。

具体的な例：レガシーなイベント追跡モジュールがPostgresqlテーブルに直書きしていた。Kafkaベースのパイプラインへの移行は40以上のコールサイトに触ることを意味していた。Branch by Abstractionで、EventTrackerインターフェースを導入し、フラグの後ろで実装を切り替え、2週間両方を走らせ、古いものを削除した。ゼロダウンタイム、緊急デプロイなし。

条件分岐の単純化：過小評価されている整理

複雑さは条件分岐に隠れる。5つのネストされたif文がある関数は、5つの条件が早期リターンか多態性で表現されるより、テスト、読み、拡張が難しい。

ネストされた条件分岐を早期リターンに置き換える：

Before:

function getDiscount(user) {
  if (user) {
    if (user.isActive) {
      if (user.plan === 'pro') {
        return 0.20;
      } else {
        return 0.05;
      }
    } else {
      return 0;
    }
  } else {
    return 0;
  }
}

After:

function getDiscount(user) {
  if (!user) return 0;
  if (!user.isActive) return 0;
  if (user.plan === 'pro') return 0.20;
  return 0.05;
}

同じロジック。読みやすさは格段に違う。早期リターンはインデント・ピラミッドを消して、各条件を独立して読める。

**条件分岐を多態性に置き換える。**複数の関数で同じ if type === X チェックが繰り返されるとき、ベースクラスやプロトコル、型ごとに1つの実装、コールサイトで実装を選ぶ。スキップしていい：2つの型しかなく、3つ目が追加される可能性がない。抽出の負荷は、二者択一で終わる選択に見合わない。

**重複条件を統合する。**3つの異なる場所で同じ条件がチェックされて、微妙に違うことをしているなら、条件を名前付き述語関数に抽出する。if (isEligibleForDiscount(user)) は3部構成のチェックを毎回繰り返すより読みやすい。

Two developers conducting code review at shared workstation

メソッドの移動：過小利用されている手法

ときに、メソッドが間違ったクラスにいる。コードが悪いのではなく、別のクラスのデータをより多く操作しているから。それは移す信号だ。

Move Methodパターンはシンプル：メソッドを特定して、実際に依存しているクラスを確認して、そこに移して、すべての呼び出し元を更新する。IDEのリントが壊れた参照を拾う。

サイドプロジェクトで最もよく現れる場所：どこに属するか明確でない、すべてが集まるユーティリティクラス。helpers.jsやutils.pyが400行に成長する。ある時点で、これらの関数は操作しているドメイン・オブジェクトに属する。Userオブジェクトに対して5つのことをする関数は、utilsでなく、Userにあるべき。

スキップする時期：プロジェクトがまだ2週間で形を探しているなら、構造を過度に設計しない。実際の使われ方から設計が浮かぶまで待つ。早くから動かすと、要件が変わるたびに動かし直すことになる。

リファクタリング前の準備：テストのベースライン

テストなしのリファクタリングはギャンブルだ。整理していると思ったら、壊していることもある。

最小限の安全網：特性化テスト。変更しようとしているコードの現在の振る舞いをキャッチするテスト。「すべきこと」ではなく、「実際にしていること」。これらのテストは回帰をキャッチするためであって、設計を検証するためではない。

ベースラインができたら、小さなコミットでリファクタリングする。パターン1つ、1コミット。何か壊れたら、何が原因か正確に分かる。3日間のリファクタリング・セッションを1つの巨大なdiffで commit する代わりに。CI が朝2時に失敗したとき、40の変更のうち何が壊したか全然分からない。

リファクタリング開始前の実践的なチェックリスト：

役に立つ制約：リファクタリングを commit メッセージで1文で説明できなかったら、おそらくやり過ぎだ。

データの構成：他のすべてを改善するパターン

複雑な条件分岐の大半は、データモデルが間違っていることが原因。生のプリミティブ型を周りで渡す代わりにドメイン・オブジェクトを使わないと、あちこちで型チェック条件が生まれる。

プリミティブをオブジェクトに置き換える（string email は EmailAddress に、int cents は Money に）は、検証ロジックの家を与える。12の場所で if not re.match(EMAIL_REGEX, email) をチェックする代わりに、EmailAddress のコンストラクタが1回してしまう。

これは Fowler の Refactoring の「Replace Type Code with Class」パターン。派手ではないが、他のほぼ何より条件分岐を減らす。

同じ原則がマジックナンバーにも適用：if status == 3 は読めない。if status == OrderStatus.SHIPPED は読める。マジックナンバーを名前付き定数に置き換えるのに5分、それ以上の見返りがある。コードを読むたびに。

リファクタリングしてはいけない場合：みんなスキップする

ほとんどのガイドはここで止まる。実際のところ：すべてのコードベースがリファクタリングに値するわけではなく、整理を「常に良い」と扱うのは別の技術的負債だ。

**ハードデッドラインの直前ではリファクタリングするな。**リファクタリングは変更を導入する。変更はリスクを導入する。48時間でクライアント・デモがあるなら、今ではない。

**削除するコードはリファクタリングするな。**フィーチャーが次のスプリントでカットされるなら、実装を整理するのは無駄な仕事だ。投資する前に削除を確認する。

**スタイルだけのリファクタリングするな。**コードが動作し、テスト済みで、誰も触らなければ、自分がそれを書き方は異なるという事実は、変更を正当化しない。回帰のリスクは実質的；利益は美的だ。

ターゲット明確なくリファクタリングするな。「整理しろ」はタスクではない。「OrderProcessorから検証ロジックを専用Validatorクラスに抽出する」はタスク。曖昧なスコープはスコープ・クリープを招き、3週間分岐が続き、競合でマージされる。

これは完璧なルール・セットではない。実行可能なもので、本番コードベースでは、実行可能は完璧より良い。

6ヶ月の段階的リファクタリングの実際

毎スプリント、少しずつ、船頭している機能に合わせてリファクタリングするチームは、作業が安い（コスト効率の高い）コードベースに行き着く。最初は速くない。だが拡張は速く、デバッグは速く、新しい貢献者のオンボーディングは速い。

ソロ・ビルダーには：利益は個人的。6ヶ月後のあなたが新しいエンジニアだ。今日整理するコードは、1月に読み直さずに理解するコードだ。

推奨システム・プロジェクトからの6ヶ月の絵：最初は1,200行のサービスファイルで、テストなし。毎フィーチャースプリント中、メソッドを抽出した。各セクションに触れるたびに、特性化テストを追加した。ドメインをより理解するにつれて、モノを正しいクラスに移動した。6ヶ月後、同じロジックが9ファイル、平均130行、すべてテスト、すべて名前付き。デプロイ頻度は週1から毎日に。バグからの復帰時間は約40%低下。

それらの数字は研究からではない。1つのチーム、1つのシステムだ。だが方向は一貫している。小さく、きれいなユニットは変更が速い。それが賭けだ。

よくある質問

コードリファクタリングとは何か、なぜ重要か
コードリファクタリングは、外部の振る舞いを変えずに、既存のコードを再構成するプロセス。コードベースを理解しやすく、拡張しやすく、デバッグしやすくする。つまり、将来のフィーチャー開発とバグ修正のコストを直接減らす。
メソッド抽出とは
メソッド抽出は、長い関数から意味のある論理ブロックを取り出して、それに名前を付けること。その名前が inline ドキュメントになる。最も一般的に使われるリファクタリング技法であり、関数が20～25行を超えるときの、その筋の最初の一手。
いつリファクタリングするべきではないか
ハードデッドラインの直前、削除されるコード、そして動作して誰も触らない機能をリファクタリングするな。リファクタリングは変更を導入するので、明確で具体的な利益がある場合のみやる価値。
Branch by Abstractionとは
Branch by Abstractionは、置き換わるコードの周りに抽象化層（インターフェースやプロトコル）を導入し、古い実装と新しい実装の両方がそれを満たす。新しいバージョンをフィーチャーフラグの後ろに置いて、並行で走らせ、安定したら古いものを削除できる。大きな切り替えのリスクを避ける。
早期リターンで条件分岐をいかに単純化するか
早期リターンはネストされたif-else ブロックを、関数トップでの条件チェックに置き換える。各条件で満たされないなら、すぐに返る。インデント・ピラミッドが消えて、ハッピーパスが一目で読める。
リファクタリング前にテストが必要か
そう。特性化テストは、変更しようとしているコードの現在の振る舞いをキャッチする。なしでは、リファクタリングは推測だ。何か壊れても本番まで分からない。テストを最初に書き、小さく、単一パターンのコミットで、リファクタリング。
Move Methodとは
Move Methodは、メソッドを、実際に最も依存しているクラスに移す。メソッドが別のクラスのデータをより多く使っているなら、それは間違った場所にいるという信号。IDEが機械的な移動と参照更新を扱ってくれる。
~/build
アイデアを生成