refactoring-optimization

リファクタリングと最適化

目的

動作を変えずに、動作しているコードの形を変えます。入力となるのは、読みづらい、変更しづらい、あるいは4か所に重複しているコードです。出力となるのは、構造を改善し、各ステップの後もテストがすべて成功し、レビュー担当者が実際に追える差分を伴う、同じ動作のコードです。

ここで重要なのは、リファクタリングの規律そのものです。関数を読みやすく書き換えるだけなら誰にでもできます。難しいのは、動作が変わっていないことを証明することです。そのため、常に同じ順序で進めます。まず安全網を用意し、小さく元に戻せる変更を1つ加えて検証し、これを繰り返します。このスキルの価値は、巧妙なリファクタリング手法を知っていることではなく、この手順を省かないことにあります。

このスキルは、2つあるうちの「適用」を担います。code-analyzer はプロジェクトを調査して問題を報告し、このスキルはその指摘の1つを受けて、実際にコードを変更します。

使用するタイミング

• 関数が、誰も頭の中で把握できないほど大きくなり、分解が必要なとき
• 同じロジックが3〜4か所にあり、それぞれに差が出始めたとき
• 到達不能または参照されていないコードがあり、誰も削除に踏み切れないとき
• 名前、マジックナンバー、またはブール値フラグのパラメーターによって、呼び出し箇所が読みにくいとき
• コードが、その意図をはるかに直接的に表現できる言語機能よりも古い書き方をしているとき
• 機能追加の前提としてリファクタリングが必要なとき。まず絡まりを解きほぐし、それから追加します

このスキルを使わないケース：

• プロジェクト全体の問題を見つける → 先にcode-analyzerを使い、その後ここに戻ります
• バグ、クラッシュ、テストの失敗を修正する → error-debuggingを使います（意図的に動作を変えるため）
• 安全網として必要な、リファクタリング用のテストを書く → 先にtest-generationを使います
• このセッションで書いたコードを整える → 組み込みのsimplifyスキルのほうが軽量です
• コードを高速化する → まず計測します。下記のパフォーマンスを参照してください

まず、より具体的なスキルを使う

複数の技術スタックには、依頼された構造変更に特化したスキルがあります。該当するものがある場合は、そのガイダンスを再実装するのではなく、該当するスキルを呼び出してください。このスキルは、安全網、順序、検証、ロールバックなど、周辺の規律に使用します。

状況	呼び出すスキル
Composeの状態配置が不適切、または状態を持つ画面とUIが混在している	compose-state-hoisting、compose-state-holder-ui-split
コンテンツスロットの代わりにブール値フラグを使っているComposeコンポーネント	compose-slot-api-pattern
Composeの修飾子チェーン、レイアウトラッパー、ハードコードされたルート	compose-modifier-and-layout-style
Composeの再コンポーズ頻度が高すぎる	compose-recomposition-performance、compose-stability-diagnostics
Kotlinのコルーチンスコープとキャンセルの構造	kotlin-coroutines-structured-concurrency
Kotlinのドメイン型におけるプリミティブ型への過度な依存	kotlin-types-value-class
Dartの機械的なリンター修正（dart fix --apply）	dart-run-static-analysis
パターンマッチに置き換えられるDartのif/elseチェーン	dart-use-pattern-matching
Flutterのウィジェットツリーの再構成、オーバーフロー、ネスト	flutter-fix-layout-issues
Flutterのレイヤー構造と状態管理の整理	flutter-apply-architecture-best-practices

Swift、Rust、C++、および通常のKotlin/JVMには専用スキルがありません。これらには、このスキルを直接使用してください。

前提条件

1. セーフティネットはありますか？ セーフティネットなしでのリファクタリングは、書き換えて運に任せるようなものです。最初の編集に着手する前に、次の3つが満たされている必要があります。

• 現時点でテストが通ること。 ベースラインがグリーンであれば、「テストが失敗するようになった」という結果に意味が生まれます。すでに失敗している場合は、いったん止めて、まず修正するか対象外にしてください。そうしないと、結果を判断する手がかりになりません。
• ワーキングツリーがクリーンで、コミット済みであること。 ロールバックは、戻す先があって初めて git checkout で行えます。未コミットの作業の上でリファクタリングすると、その作業が失われます。
• 変更対象のコードがテストでカバーされていること。 プロジェクト全体ではなく、再構成する特定の振る舞いが対象です。カバーされていない場合はその旨を伝え、何もわからないまま進めるのではなく、先に特性テストを書く（test-generation）ことを提案してください。

テストが存在せず、それでもユーザーが進めたい場合は、判断するのはユーザーです。ただし、リスクを明確に伝え、通常より手順を細かくし、コンパイラーとリンターをより頼りにしてください。

2. 使用しているスタックは何ですか？ ファイル拡張子ではなく、ビルドファイルから判定してください。リンター、慣用的な記述、参照チェックツールはビルドファイルで宣言されています。

ビルドファイル／シグナル	スタック	リファレンス
build.gradle.kts に kotlin("multiplatform") と commonMain がある	Kotlin Multiplatform ／ CMP	kotlin-kmp.md
build.gradle(.kts) に com.android.application がある	Kotlin/JVM、Android	kotlin-kmp.md
pubspec.yaml	Flutter ／ Dart	flutter-dart.md
Package.swift、*.xcodeproj、*.xcworkspace	Swift（iOS/macOS）	swift-apple.md
Cargo.toml	Rust	rust-cpp.md
CMakeLists.txt、conanfile.txt、*.cpp	C ／ C++	rust-cpp.md
package.json、pyproject.toml、go.mod、pom.xml	JS/TS、Python、Go、Java	other-languages.md

スタックのリファレンスは1つだけ読み込んでください。対象外の言語の慣用表現を混ぜると、コードベースを扱う誰にとっても違和感のある提案になります。

3. このファイルを使っている人は他にもいますか？ オープンなプルリクエストがあるファイルを大規模にリファクタリングすると、クリーンアップで得られる効果を上回るコストのマージコンフリクトが発生します。着手前に確認してください。

git log --since='2 weeks ago' --oneline -- <path>   # recent churn
git branch -a --contains HEAD >/dev/null 2>&1        # and any open branches touching it

ファイルが頻繁に変更されている場合は、大規模な変更を1回行うより、いくつかの小さなリファクタリングを順次マージすることを優先してください。

ワークフロー

ステップ 1：コードを読むのではなく、ツールで問題を見つける

どのスタックにも、そのスタック固有の問題を把握しているリンターが用意されています。手作業で読むよりも網羅的で、意見に左右されにくいものです。まずリンターを実行して、作業の方向性を決めてもらいましょう。スタックのリファレンスには、正確なコマンドと、有効にする価値のあるルールが記載されています。

次に、リンターが苦手な問題を探します。主に、変化を伴う重複と、見た目ではなく構造に問題がある箇所です。

問題	確認する点	一般的なリファクタリング
重複	5 行以上のコードが 3 か所以上にある、または少しずつ異なる類似コードがある	関数／クラスを抽出する、または差分をパラメーター化する
長い関数	変更理由が複数ある、または理解するのにコメントが必要	名前を付けた手順を抽出する
深いネスト	if／for が 3 階層以上あり、正常系の処理が一番奥にある	ガード節、早期リターン、条件の反転
長いパラメーターリスト	パラメーターが 5 個以上ある、または複数のブール値が連続する	パラメーターオブジェクトを導入する。フラグごとに関数を分割する
マジック値	説明のないリテラル、繰り返し使われる文字列キー	名前付き定数、列挙型、またはシールド型
誤解を招く名前	名前がコードの実際の動作を十分に表していない、または異なることを表している	リファレンスを考慮して名前を変更する。検索と置換は使わない
デッドコード	参照されていないシンボル、到達不能な分岐、古いフラグ	参照が本当にないことを確認してから削除する
プリミティブ型への執着	String の ID、Int の金額、あらゆる場所で使われる Long のタイムスタンプ	値型／ラッパー型
フィーチャー・エンヴィ	主に別の型のデータを操作する関数	その型に移動する

問題とリファクタリングの詳細、および各リファクタリングの進め方は refactoring-catalog.md に記載されています。問題を見ても適切な方法が 明らかでない場合に、参照してください。

ステップ 2：安全な作業から複雑な作業へと順番に進める

リファクタリングによってリスクは大きく異なり、順番によってレビュー後にどれだけの成果が残るかも変わります。 まずは機械的な作業から行いましょう。個別に検証でき、コードを減らせるうえ、構造上の問題を見えやすくし、 難しい判断も明らかになることがよくあります。

1. 自動化でき、元に戻せるもの — フォーマッター、dart fix --apply、cargo clippy --fix、IDE による名前変更。機械的にチェックでき、重要な差分に埋もれないよう、単独でマージする価値があります。
2. 局所的で影響範囲が限られるもの — 関数の抽出、定数の導入、ガード節の追加。1 つのファイル内で完結し、呼び出し元への影響はありません。
3. モジュール内の構造変更 — クラスの抽出、メソッドの移動、パラメーターオブジェクトの導入。呼び出し元も変更しますが、すべてリポジトリ内にあり、コンパイラーで検出できます。
4. 複数箇所にまたがる変更 — 公開 API の変更、プラットフォームコードの commonMain への移動、抽象化の置き換え。最初に呼び出し元を洗い出す必要があり、通常は複数の変更に分割する価値があります。

レベル 1 を超える作業に着手する前に、この計画を提示してください。ユーザーが後から却下するリファクタリング計画は無駄な作業になりますが、早い段階で方針を変えてもらえば負担は小さく済みます。

ステップ 3：一度に 1 つのリファクタリングを適用する

作業の単位は 1 つのリファクタリングであり、1 つのファイルでも 1 つのセッションでもありません。各作業の間ではコードがコンパイルでき、テストが通ります。この不変条件があるからこそ、ミスの修正にかかる時間は 1 日ではなく数分で済みます。

• 振る舞いを正確に維持してください。誰も意図していなかった振る舞いも含みます。エラーメッセージ、順序、null／空の値の扱い、オーバーフロー、別の処理が解析しているログなどです。ある振る舞いがバグだと思う場合は、それは別の変更として報告し、リファクタリングに紛れ込ませて黙って修正しないでください。
• 1 つのステップで変更するのは構造か振る舞いのどちらか一方だけにしてください。両方を含む差分はレビューできませんし、何かが壊れたとき、どちらが原因か判断できません。
• 名前変更や移動には、参照を考慮するツールを使ってください（Serena の rename_symbol、safe_delete_symbol、find_referencing_symbols、IDE のリファクタリング機能など）。テキスト検索では動的な参照を見逃し、コメントや文字列内の無関係な一致を拾ってしまいます。
• 変更していないコードは再フォーマットしないでください。無関係な 400 行の空白だけの変更によって、重要な 12 行が埋もれてしまいます。
• 変更前よりコードの一貫性を高めてください。周囲の 20 箇所の呼び出しと異なる「より良い」パターンを 1 箇所だけ使うと、コードベースは読みやすくなるどころか、読みづらくなります。

ステップ 4：各ステップの後に検証する

最後に一度だけではなく、リファクタリングのたびにテストを実行してください。スタックリファレンスにコマンドがあります。出力は失敗箇所だけに絞ってください。

テストが成功することに加えて、テストでは見つからない問題を検出するために、次の 3 点を確認してください。

• コンパイラー／リンターに警告が出ていないこと。リファクタリング後に新しい警告が出た場合、何かが中途半端に移動された可能性があります。
• 差分が意図したものだけであること。次に進む前に git diff を確認してください。意図しない変更はよくあるもので、この段階なら簡単に見つかりますが、レビューで見つけるのは大変です。
• パブリック API が変更されていないこと。変更された場合は、意図的なものであり、すべての呼び出し元が更新されていることを確認してください。公開済みのライブラリでは、リポジトリ外の呼び出し元をコンパイラーでは見つけられません。バイナリー互換性を確認するツールについて、スタックリファレンスを確認してください。

テストに失敗した場合は、その先にデバッグを進めるのではなく、そのステップを元に戻してください。ステップは最初から小さく分けてあるため、修正するより、正しくやり直すほうが低コストです。デバッグが必要になるようなリファクタリングは、リファクタリングではありません。

ステップ 5：変更したことと、変更しなかったことを報告する

## Refactorings applied
<one line per refactoring: what moved/merged/was deleted, and why it is better>

## Verification
<command run, and the real result — after which step>

## Behavior preserved
<how you know: tests, compiler, caller inventory>

## Left alone
<smells found but not addressed — as `file:line` — with the reason>

## Found while refactoring
<bugs or surprises discovered, reported not fixed>

価値が積み重なるのは、最後の 2 つのセクションです。バグも黙って修正したリファクタリングは誰もレビューできない変更です。また、意図的に残したコードの問題点は、次の担当者に必要な情報です。

最適化について

「最適化」には 2 つの異なる意味があり、それぞれ異なる慎重さが必要です。構造の整理、つまり上記の作業は安全で、元に戻せます。一方、パフォーマンスの改善はそうではありません。速度のために読みやすさを犠牲にし、時間がどこにかかっているのかを見誤ることもよくあります。

そのため、速度に関する依頼を受けたら、まず測定してください。プロファイルを取り、実際に時間のかかっている箇所を特定し、その箇所だけを変更して、もう一度測定してください。変更前後の数値がない最適化は、効果が証明されていない読みやすさの低下です。各言語のプロファイラーをスタックリファレンスに記載しているのも、まさにこのためです。ホットループ内のアルゴリズム計算量の問題はすぐに修正する価値があります。それ以外は、データが得られるまで保留してください。

操作

操作	ユーザーの意図	出力
refactor（デフォルト）	「ここを整理して」「この関数をリファクタリングして」	動作を維持する編集を、段階的に検証しながら実施します
plan	「最初に何をリファクタリングすべきか」	各ステップのリスクを示した、優先順位付きの実行計画です。編集は行いません
duplicates	「ここにはコピペがたくさんあります」	重複箇所を特定し、効果が見込める場合は共通の抽象化を取り出します
simplify	「この関数は複雑すぎます」	名前を付けたステップに分解し、ネストを浅くします
deadcode	「使われていないコードを削除して」	参照を検証したうえで削除し、動的参照に関する注意点も明記します
rename	「この名前は紛らわしいです」	宣言箇所と呼び出し箇所を横断して、参照を考慮した名前変更を行います
modernize	「慣用的な書き方にして」	現在の言語バージョンに合わせて、リンターを活用した慣用表現への整理を行います

重要な制約

• リファクタリング中に動作を変えてはいけません。 これが、このプラクティスの土台となる唯一のルールです。 動作が変わる可能性がある場合、それは書き換えであり、個別にレビューとテストを行う必要があります。
• ベースラインが失敗状態、または不明な状態でリファクタリングしてはいけません。 成功している開始点がなければ、失敗の原因を特定できず、単に明らかになっただけのバグまでリファクタリングのせいにされます。
• 参照されていないと証明できないコードを削除してはいけません。 リフレクション、依存性注入、シリアライズ、プラットフォームのエントリポイント、ビルドフレーバー固有のコードはテキスト検索では見つかりません。スタック別のリファレンスには、それぞれが隠すものが記載されています。
• 同じ変更でリファクタリングと機能追加を行ってはいけません。 レビュアーは両者を切り分けられず、git bisectでも同様です。
• 生成コードに手を加えてはいけません。 ジェネレーターまたはテンプレートを修正してください。再生成された出力が編集を上書きし、差分が誤解を招きます。
• 2か所にしかない重複から抽象化を取り出してはいけません。 誤った抽象化を作るより重複を残すほうがコストは低く、実際に変化する部分を明らかにするのは3つ目の出現箇所です。
• 測定せずに最適化してはいけません。 上記を参照してください。測定を伴わないパフォーマンス改善は、たいていコードの明瞭さを損なう一方で、何も得られません。
• 指標を追いかけてはいけません。 循環的複雑度や行数は、確認する価値のあるコードを示す目安であり、目標ではありません。コードを8つの関数に分割しても、全体として悪化することがあります。
• リポジトリ外の利用者も含めた呼び出し元の一覧なしに、公開 API の構造を変更してはいけません。 リポジトリ内の利用者はコンパイラーで把握できますが、リポジトリ外の利用者は把握できません。
• 「適切な」構造が判断を要する場合は立ち止まって相談してください。 レイヤー構成、モジュール境界、抽象化の選択には意図が反映されますが、このスキルだけではコードからその意図を読み取れません。

リファレンス

• refactoring-catalog.md — コードの問題点とリファクタリングの対応表、各リファクタリングの手順、特性テスト、ロールバック
• kotlin-kmp.md — Kotlin と Android の慣用表現、detekt/ktlint、KMP の commonMain への引き上げと expect/actual の整理、Compose のルーティング、バイナリ互換性
• flutter-dart.md — dart analyze/dart fix、ウィジェットの抽出とヘルパーメソッドの使い分け、const コンストラクター、破棄処理、ネスト
• swift-apple.md — SwiftLint、プロトコルの抽出、guard による早期リターン、SwiftUI ビューの分解、@MainActor と並行処理の衛生管理
• rust-cpp.md — cargo clippy と --fix、イテレーターチェーン、ネストした match より ? を使う方法、clone()/unwrap() の削減、RAII、スマートポインター、const の正確性、clang-tidy
• other-languages.md — プロジェクトで必要となる場合に備えた、JS/TS、Python、Go、Java のツールと慣用表現