読みにくいコードを直す練習:名前・分割・重複の順に見る

初級 | 14分 で読める | 2026.07.10

公式ドキュメント

可読性リファクタリングは、外から見える動作を保ったまま、次に変更する人が目的・境界・業務ルールを追いやすい構造へ直す作業です。 読みやすいコードに唯一の正解はないため、短さや好みではなく「変更時に迷う場所」を探します。

なぜ順番が必要か

名前、分割、抽象化を一度に変えると、どの変更で動作が変わったか分かりません。先に代表ケースを固定し、名前で目的を表し、異なる変更理由を分け、最後に重複を検討すると、各段階の効果を説明できます。

リファクタリング中に仕様の誤りを見つけても、動作変更は別のテストと変更として扱うと差分を検証しやすくなります。

登場人物と対象

現在の実装者、レビュー担当、将来変更する保守担当、仕様を確認できるプロダクト担当が関わります。対象は名前、関数境界、条件分岐、重複、データ構造、テストです。性能・公開API・エラー形式が外部契約なら、見た目が同じでも変更してはいけません。

ワークショップの流れ

  1. 現在の入出力と副作用を確認する。
  2. 正常、境界、無効入力のテストを用意する。
  3. 一度に一種類だけ変更する。
  4. テストと差分を確認する。
  5. 読み手が業務ルールを見つけられるかレビューする。
  6. 性能やログなど非機能の挙動も必要に応じて比較する。

練習前のコード

function f(a) {
  let x = 0;
  for (const v of a) {
    if (v.active === true) {
      x = x + v.price * v.count;
    }
  }
  if (x >= 5000) {
    x = x - 500;
  }
  return x;
}

動きますが、名前だけでは目的が分かりません。

1. 名前で目的を表す

function calculateActiveItemTotal(items) {
  let total = 0;

  for (const item of items) {
    if (item.active) {
      total += item.price * item.count;
    }
  }

  if (total >= 5000) {
    total -= 500;
  }

  return total;
}

変数名と関数名だけで、処理の輪郭が見えるようになりました。

2. 異なる変更理由を分ける

集計と割引は、別々の理由で変更されます。

function calculateActiveItemTotal(items) {
  const subtotal = items
    .filter((item) => item.active)
    .reduce((sum, item) => sum + item.price * item.count, 0);

  return applyOrderDiscount(subtotal);
}

function applyOrderDiscount(subtotal) {
  return subtotal >= 5000 ? subtotal - 500 : subtotal;
}

割引条件だけをテストしやすくなります。

3. 短さより読み手を優先する

メソッドチェーンへ変えれば必ず読みやすいわけではありません。チームが for...of に慣れているなら、最初の改善版の方が読みやすい場合もあります。

判断では次を確認します。

  • 関数名から役割が分かるか
  • 業務ルールが見つけやすいか
  • テストしたい単位に分かれているか
  • 変更時の影響範囲を説明できるか
  • 初めて読む人が途中の値を追えるか

改善パターンの比較

症状最初の候補注意点
名前が抽象的意図を表す改名スコープ以上に長くしない
関数に複数の理由関数抽出呼び出しを追わせすぎない
条件が深いガード節・条件名評価順を変えない
重複がある共通知識を抽出偶然似た処理を統合しない
引数が多いデータ構造を見直す不要な結合を増やさない

重複している文字列ではなく、同じ理由で一緒に変わる知識をまとめます。 似て見えても割引と送料のルールは別々に変わるなら、無理に共通化しません。

4. 動作を守る

リファクタリングは、外から見える動作を変えずに内部を整える作業です。変更前に代表ケースのテストを用意します。

assert.equal(calculateActiveItemTotal([]), 0);
assert.equal(calculateActiveItemTotal([{ active: true, price: 1000, count: 2 }]), 2000);
assert.equal(calculateActiveItemTotal([{ active: true, price: 3000, count: 2 }]), 5500);

一度に名前、構造、仕様を変えず、小さく確認します。

この例では空配列、割引なし、しきい値を超える入力を確認しています。実際の仕様では、ちょうどしきい値、非アクティブ項目、0個、負数を許すかも確認します。架空の業務ルールをテストへ追加せず、仕様の正本に照らします。

ケーススタディ:返品を含む注文金額

実務では、単純な集計に返品、会員割引、送料、税計算が加わり、一つの関数へ条件が積み重なります。たとえば注文項目の数量が正なら購入、負なら返品を表し、会員割引は商品の小計だけに適用し、送料には適用しない仕様を考えます。ここで全項目を正数へ直してから合計する「整理」は、見た目が簡潔でも返品を購入として数えるため、リファクタリングではありません。

最初の判断は、現在の動作のうち何が契約かを列挙することです。返品は小計を減らす、割引判定は返品反映後に行う、送料は割引対象外、計算結果の端数処理は最後に一度だけ行う、という順序を仕様と既存テストから確認します。仕様書とコードが食い違う場合は、勝手に「正しい」側へ直さず、現在の観測結果を特性テストへ固定し、仕様変更のIssueを分けます。

次に名前を直します。xdiscountableSubtotal、数量が0未満か調べる条件を isReturnedItem のように変え、途中値をログへ出さなくても式の意味を追える状態にします。その後、商品小計、会員割引、送料を別関数へ分けます。ただし、端数処理まで各関数へ移すと丸め回数が増えて結果が変わるため、注文合計の境界に残します。重複が見えても、会員割引とキャンペーン割引は適用条件や担当部署が別なら一つの汎用割引関数へ急いで統合しません。

何を観測するか

入力と最終金額だけでなく、中間の意味が保たれているかを観測します。通常購入、返品混在、割引しきい値の直前・一致・直後、送料あり・なし、端数が出る価格を表形式で用意し、変更前後の結果を同じ入力で比較します。外部から見えるログが運用監視に使われているなら、ログの件数、項目名、出力順も比較対象です。性能が重要な一括計算なら、同じ件数の注文を複数回処理し、中央値を記録します。一回だけ速かった結果は根拠にしません。

失敗条件は、金額の不一致だけではありません。例外の種類が変わる、無効入力を黙って受け入れる、項目の処理順が変わる、監査ログが欠ける、関数を細分化した結果として業務ルールを探すため五つ以上のファイルを往復する、といった状態も改善失敗です。テストが通っても、名前が実際の範囲より広い、抽出した関数が呼び出し元の変数へ依存する、同じ条件が複数箇所へ残るなら、次の変更者の迷いは減っていません。

良い改善と悪い改善

悪い例は、処理を一行のメソッドチェーンへ圧縮し、返品判定、割引、端数処理を無名関数の中へ埋める変更です。行数は減っても、割引対象の範囲と計算順を読む負担が増えます。さらにテストなしで Math.abs(quantity) を使えば、現在の返品動作まで変えてしまいます。

良い例は、calculateMerchandiseSubtotalapplyMemberDiscountaddShippingFee のように業務語で境界を表し、注文全体の関数には計算順が上から読める形で残す変更です。各関数の入出力は小さくし、端数処理は仕様どおり最後に一度だけ行います。差分は「改名」「関数抽出」「重複判断」の順に分け、その都度同じ特性テストを実行します。読みやすさを感想だけで決めず、レビュー担当が割引対象と返品の扱いをコードから説明できるかで確かめます。

続行・中止の判断手順

一段階ごとに、変更目的を一文で言えるか、外部動作の比較手段があるか、差分が一種類に収まっているかを確認します。三つとも満たすなら続行し、結果をテストと差分で観測します。仕様が不明、比較できるテストがない、性能やログの契約が分からない場合は、抽象化を進めず調査へ戻ります。

また、抽出後の関数名が「処理する」「データを扱う」のように曖昧なら、責任の境界がまだ見えていない合図です。二つの重複が将来も同じ理由で変わると説明できない場合も共通化を止めます。中止は失敗ではなく、動作と理解可能性を守る判断です。

よくある誤解

関数を短くすれば必ず読みやすいわけではありません。小さすぎる関数が連鎖すると処理全体を追いにくくなります。コメントを消すことも目的ではありません。「何をするか」をコードで表せても、制約の理由はコメントや設計記録が必要な場合があります。また、抽象化は重複を一度見ただけで導入せず、変更理由が同じかを確認します。

注意とベストプラクティス

  • 変更前にGit差分が把握できる状態にする。
  • 公開API、例外、ログ、順序、副作用を外部動作として扱う。
  • フォーマットだけの変更と構造変更を分ける。
  • 一段階ごとにテストを実行し、レビュー可能な差分にする。
  • 性能が重要な処理は変更前後を同条件で測る。

デバッグ・確認方法

失敗したら最後の小さな変更へ戻り、入力、出力、副作用のどれが変わったか確認します。テストが不足していた場合は、先に現在の正しい動作を固定します。差分で削除された条件や早期returnを確認し、静的解析、単体テスト、結合テストを対象範囲に応じて実行します。

まとめ

可読性改善は、名前、責任の分割、条件、重複を順に見て、各変更後に外部動作を確認する作業です。 短さや抽象化の多さを競わず、業務ルールと変更箇所を次の読み手が見つけやすいかで判断します。

参考リソース

次に読む記事

← 一覧に戻る
PR
PR
PR
PR