リファクタリング

概要

リファクタリングとは、外から見える振る舞いを変えずにコードの内部構造を変えることです。コードベースは変更を受け入れるたびに構造がずれていくため、リファクタリングは変更容易性、すなわち次の変更を安く安全に受け入れられる性質を回復し続ける仕事です。リファクタリングは特別なプロジェクトでも、許可を待つフェーズでもありません。いつ実施してもよく、そしていつも求められています。規律が必要なのは「いつやるか」ではなく「どうやるか」です。レビュアーが検証できる単位で、取り消せる手順で、最終的には部品を消せる構造に向かって進めます。このページでは、その規律を与える3つの実践、すなわち構造変更と振る舞い変更の分離、小さく可逆な手順、削除を設計する視点と、そのすべての前提になるテストコードの役割を解説します。

構造変更と振る舞い変更

コードへの変更はすべて2つのカテゴリのどちらかに属し、両者は検証の仕方がまったく異なります。

	構造変更	振る舞い変更
変わるもの	名前、境界、依存関係、重複といったコードの整理のされ方	ソフトウェアが外から見せる動き
気づく人	コードを読む開発者だけ	ユーザー、APIの利用者、テスト
検証方法	既存のテストが変更なしで通ること	更新されたテストが新しい振る舞いを検証すること
リスク	低く、機械的に確認できる	定義上プロダクトのリスクを伴う

2つの違いを、注文履歴を取得して表示用に整形する次のフックを共通の例として見ていきます。

共通の例：注文履歴の取得と整形

// 注文履歴を取得し、表示用に整形して返すフック
export const useOrderHistory = () => {
  const { data } = useOrders();

  // キャンセル済みの注文を除き、新しい順に並べる
  const orders = (data ?? [])
    .filter((order) => order.status !== "cancelled")
    .sort((a, b) => compareDesc(a.orderedAt, b.orderedAt));

  return { orders };
};

このフックへの変更が2つのカテゴリのどちらに属するかは、一覧の内容（外から見える振る舞い）が変わるかどうかで決まります。構造変更では、コードの配置が変わるだけでフックが返す一覧は同一のため、既存のテストが変更なしで通ります。

構造変更：整形ロジックを関数として切り出す

const selectOrders = (data: Order[] | undefined) =>
  (data ?? [])
    .filter((order) => order.status !== "cancelled")
    .sort((a, b) => compareDesc(a.orderedAt, b.orderedAt));

export const useOrderHistory = () => {
  const { data } = useOrders();
  return { orders: selectOrders(data) };
};

振る舞い変更では、一覧の内容が変わるため、テストが新しい振る舞いに更新されます。

振る舞い変更：除外をやめてキャンセル済みも一覧に表示する

export const useOrderHistory = () => {
  const { data } = useOrders();

  // 除外の filter を消し、新しい順の並べ替えだけを残す
  const orders = (data ?? [])
    .sort((a, b) => compareDesc(a.orderedAt, b.orderedAt));

  return { orders };
};

リファクタリングは定義上、このうち構造変更だけを指します。変更がソフトウェアの動きを変えた瞬間、それはもうリファクタリングではなく、振る舞い変更としての精査を受けるべき変更になります。

2つを1つのプルリクエストで混ぜない

構造変更と振る舞い変更を同じプルリクエストに入れてはいけません。理由はそれぞれのレビューの仕方にあります。構造変更だけのプルリクエストは「振る舞いは同一か」というただ1つの問いで検証できます。証拠は機械的です。テストがひとつも変更されておらず、すべて通っていることです。テストが難しい問いに答えてくれるため、レビュアーは大きな構造の差分でも素早く承認できます。振る舞い変更のプルリクエストは意図に照らしてレビューされます。差分には更新されたテストが含まれ、レビュアーは新しい検証内容が仕様と一致しているかを確認します。構造変更と振る舞い変更が混ざったプルリクエストは、この両方の検証戦略を壊します。テストが変更されているため「テストが変更なしで通っている」ことはもはや振る舞いの保存を証明せず、レビュアーはすべての行について、それが振る舞いを変えるはずの行なのかどうかを手作業で分類することになります。レビューは遅くなり、同時に信頼性も下がります。先ほどの切り出しを例にすると、2つの扱いは次のような差になります。まず、プルリクエストで切り出しだけを行うパターンです。テストにはひとつも変更が入らず、そのまま通ること自体が振る舞いの保存の証拠になります。キャンセル済みを表示する変更は、更新されたテストを伴う後続の振る舞い変更プルリクエストとして出します。

const selectOrders = (data: Order[] | undefined) =>
  (data ?? [])
    .filter((order) => order.status !== "cancelled")
    .sort((a, b) => compareDesc(a.orderedAt, b.orderedAt));

export const useOrderHistory = () => {
  const { data } = useOrders();
  // 切り出し前と同じロジックを呼ぶため、フックが返す一覧は変わらない
  return { orders: selectOrders(data) };
};

// テストには手を付けない。変更なしで通ることが
// 「振る舞いが保存されている」ことの証拠になる
test("注文履歴を新しい順で返す", () => {
  const { result } = renderHook(() => useOrderHistory());

  // キャンセル済みの注文は一覧に含まれない
  expect(result.current.orders).toEqual([delivered2, delivered1]);
});

一方、切り出し（構造変更）と表示対象の変更（振る舞い変更）を1つのプルリクエストに混ぜると、実装と同じ差分の中でテストの期待値まで書き換わります。「テストが変更なしで通る」という構造変更の証拠が失われ、レビュアーは全行を手作業で分類することになります。

const selectOrders = (data: Order[] | undefined) =>
  (data ?? [])
    // 切り出しと同時に除外の filter を消し、
    // キャンセル済みまで表示している
    .sort((a, b) => compareDesc(a.orderedAt, b.orderedAt));

export const useOrderHistory = () => {
  const { data } = useOrders();
  return { orders: selectOrders(data) };
};

  test("注文履歴を新しい順で返す", () => {
    const { result } = renderHook(() => useOrderHistory());

-   // キャンセル済みの注文は一覧に含まれない
-   expect(result.current.orders).toEqual([delivered2, delivered1]);
+   // キャンセル済みの注文も一覧に含まれる
+   expect(result.current.orders).toEqual([delivered2, cancelled1, delivered1]);
  });

差し戻しの単位も同じように劣化します。プルリクエストは単独で差し戻せる単位であるべきです（タスク分解を参照してください）。混ざったプルリクエストの差し戻しは、誤った振る舞い変更を取り消すために正しい構造変更まで捨てることになり、逆もまた同様です。

振る舞い変更のプルリクエストでレビュアーからリファクタリングを求められたとき、そのプルリクエストの中で直す必要はありません。指摘を受け入れたうえで、後続の構造変更だけのプルリクエストとして実施すれば、両方の変更が検証可能なまま保てます。

タイミングは選択であって規則ではない

構造変更と振る舞い変更は別のプルリクエストにしますが、時間的な順序は純粋な選択です。

先に構造：いまの構造がこれからの振る舞い変更を不格好にするなら、先にコードを整えます。その後の振る舞い変更は小さくなり、レビューしやすくなります。
後で構造：振る舞い変更を急ぐときや、作業して初めて構造の問題が見えたときは、振る舞いを先に届けて直後に整理し、次の変更に備えた状態にしておきます。
やらない：めったに触らないコードでは、構造を直すコストが回収されないことがあります。手を付けない判断も正当な選択肢です。

選択の実用的な判断材料は、そのコードがどれくらいの頻度で変更されるかです。多くの変更が通る場所の構造はすぐに元を取り、誰も再訪しない場所の構造は永遠に元を取りません。また、観測可能な振る舞いを早く届けること自体が不確実性を減らします。振る舞いが検証される前に構造を磨き込むことは、生き残らないかもしれないコードの最適化です。

「振る舞いが変わっていない」を定義するのはテスト

リファクタリングとテストコードは切り離せない関係にあります。構造変更の検証方法である「既存のテストが変更なしで通ること」は、テストが観測可能な振る舞いを十分に検証していて初めて成立します。つまり、リファクタリングが振る舞いを保存しているという主張は、振る舞いを検証しているテストの強さまでしか信用できません。リファクタリング中は次の2つのシグナルに注意します。

テストを変更せざるを得なかった。その変更が実は構造だけの変更ではなかったか、テストが観測可能な振る舞いではなく実装の詳細を検証しているかのどちらかです。前者は差分を、後者はテストを直す必要があります（テストコードを参照してください）。
途中でロジックを壊したのにテストが落ちなかった。中間状態の誤りでテストが1つも落ちなかったなら、リファクタリングが触れている領域の安全網に穴があります。

小さく可逆な手順

変更のコストは、差分の大きさに比例するのではなく、それ以上の速さで膨らみます。大きな差分はレビューに不釣り合いに長い時間がかかり、並行する作業との競合が増え、誤りをうまく隠します。そのためリファクタリングは、システムを常に動く状態に保つ小さなステップの連続として進めます。ステップのリズムは固定です。構造の変更を1つ加え、すべてが通ることを確認し、コミットします。名前の変更、関数の抽出、移動はそれぞれが検証済みの1ステップとして積み上がります。誤りだったステップは小さなコミットを1つ差し戻すだけで取り消せ、1日分の他の編集からほどく必要はありません。同じ規則はプルリクエストの単位にも当てはまります。構造変更だけの小さなプルリクエストをデフォルトブランチへ継続的にマージし、長生きするリファクタリング専用ブランチに積み上げないようにします。長生きするブランチはあらゆる並行変更との競合を溜め込み、最終的なマージは小さなステップが避けようとしていた大きくリスクの高い差分そのものになります。個々のプルリクエストは、レビュアーが数分で読み終えられる大きさに保ちます。 1回の安全なステップには大きすぎる構造変更は、並行変更（parallel change）として実施します。新しい構造を古い構造の隣に作り、利用側を段階的に移していく方法です。価格表示を多通貨対応の関数に置き換える例で、3つのステップを見ていきます。

拡張する

新しい構造を古い構造の隣に、まだ何も使っていない状態で追加します。振る舞いには一切触れないため、このステップは自明に安全です。

拡張：新しい関数を古い関数の隣に追加する

// 既存の関数には触れない
export const formatPrice = (price: number) =>
  `¥${price.toLocaleString()}`;

// 新しい関数を追加する。まだどこからも使われていない
export const formatMoney = ({ amount, currency }: Money) =>
  `${amount.toLocaleString()} ${currency}`;

移行する

利用側を古い構造から新しい構造へ、少しずつ移します。個々の移行は小さく単独で差し戻せるプルリクエストであり、システムはその間のどの時点でも動き続けます。

移行：利用側を1か所ずつ新しい関数へ移す

- {formatPrice(order.total)}
+ {formatMoney({ amount: order.total, currency: order.currency })}

収縮する

古い構造を使うものがなくなったら、古い構造を削除します。このステップが終わるまでリファクタリングは完了していません。両方の構造を抱えたコードベースは、リファクタリングを始める前より変更しにくいからです。

収縮：使われなくなった古い関数を消す

- export const formatPrice = (price: number) =>
-   `¥${price.toLocaleString()}`;

このパターンは所有権の境界をまたいでも機能します。APIレスポンスのフィールド名の変更も同じ拡張、移行、収縮の順序に従い、提供側と利用側がそれぞれ単独で安全なステップとして進められます。

削除を設計する

多くのリファクタリングは削除で終わります。収縮のステップは古い構造を消し、生き残らなかった実験は取り除かれ、ロールアウトを終えたFeature Flagは撤去されます（リリース戦略を参照してください）。削除は変更の分類と設計が出会う場所です。コードを消すこと自体は簡単ですが、安全に、早く、簡単に消せることは、そのコードが書かれた時点で与えられたか与えられなかったかが決まる性質です（変更容易性を参照してください）。依存元を機械的に列挙でき、関係する部品がまとまって置かれているコードは消しやすいコードです。これは凝集度が高く（一緒に変わるものが1か所にまとまっている）、結合度が低い（関係のないもの同士が依存し合わない）構造の言い換えであり、リファクタリングが構造をどちらへ動かすべきかの指針でもあります。凝集度には強さの分類があり、部品をまとめている理由が本質的であるほど強くなります。

凝集度	まとまりの理由	典型例
機能的凝集（最も強い）	1つの目的を果たすために必要なものが集まっている	1つの機能を構成するUI、ロジック、テストのサブツリー
逐次的凝集	前の処理の出力を次の処理が受け取る	取得から変換、整形へと流れる一連の処理
通信的凝集	同じデータを扱う	同じエンティティを読み書きする一連の処理
手続き的凝集	実行の順序だけを共有する	画面表示前に順番に実行されるチェック処理
時間的凝集	同じタイミングで実行される	起動時にまとめて呼ばれる初期化処理
論理的凝集	種類が似ている	フラグで分岐する汎用関数、種類だけでまとめたユーティリティ
偶発的凝集（最も弱い）	たまたま同じ場所にある	雑多なものが集まった`common`モジュール

優先順位は明確です。機能的凝集を目指し、逐次的凝集と通信的凝集までを許容し、論理的凝集と偶発的凝集は分解の対象とします。コードをまとめている理由が「一緒に変わるから」ではなく「種類が似ているから」や「たまたまそこにあったから」になっていたら、それが弱い凝集のサインです。凝集度を高く、結合度を低く保つ実践は次のとおりです。

依存関係を明示的かつ一方向に保つ。「これに依存しているのは何か」という問いには、型、import、モジュール境界から機械的に答えられなければなりません。依存元を列挙できないコードは自信を持って消せません。
種類ではなく概念でコードをまとめる。ある機能に必要なものが1つのサブツリーに収まっていれば、機能の削除はサブツリーの削除です。共有レイヤーに散らばっていれば、削除のたびに調査が始まります。種類だけでまとめられたutilsやcommonといった置き場は、論理的凝集や偶発的凝集の典型です。
共有抽象を削除リスクとして扱う。共有ヘルパーの利用者が1つ増えるたびに、そのヘルパーは消しにくく、変えにくくなります。共有化は重複が負担だと証明されてから行い、先回りでは行いません。
死んだコードは残さず消す。コメントアウトされたブロック、使われていないexport、切り替わることのないフラグはただではありません。読み手を誤解させ、変更検知を薄めます。履歴はバージョン管理がすべて保存しており、消したコードの復元は差し戻し1回です。

種類ではなく概念でまとめる差は、ディレクトリ構造にもっとも分かりやすく表れます。

種類でまとめた構造と概念でまとめた構造

# ❌ 種類でまとめる：注文履歴を消すには全レイヤーの調査が要る
src/
  components/OrderList.tsx
  hooks/useOrderHistory.ts
  utils/orderFormatters.ts

# ✅ 概念でまとめる：注文履歴の削除はサブツリーの削除で済む
src/
  features/order-history/
    OrderList.tsx
    useOrderHistory.ts
    formatters.ts

この実践を将来に向けると、新しいコードを書くときの問いになります。これはどうやって消されるのか、という問いです。答えを持つモジュール、すなわち依存元を列挙でき、部品がまとまっていて、内部に手を突っ込まれていないモジュールは安く消せて、まったく同じ理由で安く変えられます。

変更容易性

リファクタリングが安く保ち続ける対象である、変更の3分類（分ける・なおす・壊す）を解説します。

タスク分解

単独で差し戻せる単位への分割という、構造と振る舞いの分離と同じ原則を扱います。

テストコード

「振る舞いが変わっていない」を願望ではなく検証可能な主張にするテストを解説します。

プルリクエスト

1つのプルリクエストを1つの意図に集中させ、レビューを速く確実に保ちます。

​概要

​構造変更と振る舞い変更

​2つを1つのプルリクエストで混ぜない

​タイミングは選択であって規則ではない

​「振る舞いが変わっていない」を定義するのはテスト

​小さく可逆な手順

​削除を設計する

​関連ページ

変更容易性

タスク分解

テストコード

プルリクエスト

概要

構造変更と振る舞い変更

2つを1つのプルリクエストで混ぜない

タイミングは選択であって規則ではない

「振る舞いが変わっていない」を定義するのはテスト

小さく可逆な手順

削除を設計する

関連ページ