Refactoring discipline

Refactoring Discipline

構造変更（Tidying/Refactoring）と振る舞い変更（機能追加・バグ修正）を絶対に混ぜない。混ぜた瞬間、レビュアーも自分自身も「何が原因で壊れたか」を追えなくなる。

鉄則（Iron Law）

1. 1回の変更は「構造のみ」か「振る舞いのみ」のどちらか一方に限定する。両方が必要なタスクは必ず2ステップ以上に分割する。
2. 構造変更のコミット・差分では、既存テストの期待値（アサーションの中身）を一切変更しない。テストが全部green のまま構造だけが変わっていることを確認する。
3. 「ついでに直す」を禁止する。作業中に見つけた無関係な改善点は、その場で直さずメモに残し、別タスクとして提案する。
4. 差分は小さく保つ（目安100行、1000行は基本的に大きすぎる）。大きくなりそうなら着手前に分割案をユーザーに提示する。

作業手順

ステップ0: 分類する

着手前に自問する。

• これは構造変更のみか？ → Tidyingとして進める（下記チェックリスト）
• これは振る舞い変更（機能追加・バグ修正）か？ → 構造は一切いじらず実装する
• 両方必要に見える場合 → 「まず構造を整えてから機能を足す」の順で2ステップに分解する。多くの場合、先に構造を整えると機能追加が楽になる（これがTidy Firstの本質）。

ステップ1: 構造変更のみを行う場合（Tidying）

1. 対象コードを保護する既存テストがあるか確認する。なければ先に「現状の振る舞いを固定するテスト」を書く（振る舞いを変えない証拠を残すため）。
2. 変更を「数分〜数時間で終わる粒度」に分解する。1回のTidyingにつき1つのスメル・1つの手法だけを適用する。
3. 各ステップの後、必ずテストを実行し全green を確認してから次のステップに進む。
4. 「今このリファクタリングをする価値があるか」を判断する。今すぐ必要な変更に関係のない箇所まで手を広げない（オプション性・時間的価値で判断）。

ステップ2: 振る舞い変更を行う場合

1. 構造には触れない。既存の設計が多少読みにくくても、この場では我慢する。
2. どうしても構造変更が必要になったら、いったん手を止め、構造変更を先出しの別ステップとして切り出す。
3. 実装が終わったら、その差分が本当に「振る舞い変更のみ」かを見直す（変数名を勝手に変えていないか、無関係な整形をしていないか）。

ステップ3: コミット・PR単位を検証する

• 1つの変更は1つの自己完結した目的を持つか？
• リファクタリングと機能追加が同じコミットに混在していないか？
• 大きな機能追加は水平分割（レイヤーごと）または垂直分割（機能ごと）で複数の小さい変更に分けられないか？

コードスメル→対応リファクタリング 対応表

スメル	兆候	対応リファクタリング
Long Method	スクロールしないと全体が見えない	Extract Function
Long Parameter List	引数が3つ以上	Introduce Parameter Object / Extract Class
Duplicated Code	同じ断片が3箇所以上	Extract Function + 呼び出し側統一
Feature Envy	あるクラスのメソッドが他クラスのデータばかり使う	メソッドをそのデータのクラスへ移動
Data Clumps	常に一緒に渡される変数群	オブジェクトにまとめる
Large Class	責務が複数混在	Extract Class

適用手順：

1. 該当するスメルを1つ特定する。
2. 保護テストの有無を確認する（なければ先に書く）。
3. 対応する定型リファクタリングだけを適用する。都度考えて独自のやり方を編み出さない。
4. 適用後、テストがgreenのままであることを確認する。

フィーチャーフラグの除去・早期軌道修正

• 役目を終えたフィーチャーフラグ（リリース完了後のRelease Toggle、決着済みの実験フラグ）の削除はそれ単体で1つの独立した変更として扱う。機能追加やバグ修正と同じコミットに混ぜない（Uber は piranha で自動棚卸しするほど、放置フラグを負債として扱う）。
• 実装の途中で「この方針は間違っていた」と気づいたら、書きかけの差分に固執せず早期に破棄してやり直す。小さい差分で進めていれば破棄のコストも小さい。継ぎ足しで方針転換すると構造と振る舞いの変更が必ず混ざる。

チェックリスト（着手前・完了前）

着手前：

• この変更は構造のみか、振る舞いのみか、明確に切り分けたか
• 保護テストは存在するか、なければ先に書いたか
• 差分の見込みサイズは100行程度に収まりそうか

完了前：

• テストの期待値（アサーション自体）は一切変えていないか（構造変更の場合）
• 「ついでに直した」箇所が紛れ込んでいないか
• コミット・差分の説明が「1つの自己完結した目的」で言い切れるか
• 無関係なフォーマット変更・変数名変更が混入していないか

アンチパターン集

やりがちな思考	現実
「ついでにこの変数名も直しておこう」	ついでの変更はレビュー範囲を曖昧にし、バグ混入時の切り分けを不可能にする。別コミットにする
「動いたし、リファクタリングとバグ修正を一気に片付けよう」	動いたことは振る舞いが変わっていないことの証明にならない。テストの期待値を変えていないか別途確認する
「テストがないけど、小さい変更だから大丈夫」	小さい変更ほど「保護テストなしで壊した」ことに気づきにくい。先に固定テストを書く
「せっかくだから全部きれいにしよう」	スコープ拡大は差分を肥大化させ、レビュー不能にする。今のタスクに必要な範囲だけ整える
「このスメルは特殊だから独自のやり方で直そう」	大抵は既知のリファクタリングカタログで対応済み。パターンマッチングで機械的に処理する方が安全
「テストを直せば済む」（構造変更でテストが落ちたとき）	構造変更でテストが落ちるのは、振る舞いが変わった証拠。テストを合わせるのではなく実装を疑う

出典

• Kent Beck, Tidy First? — 構造変更と振る舞い変更の分離、Tidyingの粒度に関する原則
• Martin Fowler, Refactoring: Improving the Design of Existing Code — コードスメルと対応リファクタリングカタログ（Chapter 3: Bad Smells in Code）
• Google Engineering Practices — Small CLs（変更を小さく保つ原則、CC-BY 3.0）
• Uber Engineering — Piranha（不要フィーチャーフラグの自動削除、Apache-2.0）・Martin Fowler Feature Toggles（フラグ削除の規律）