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

初級 | 11分 で読める | 2026.07.10

公式ドキュメント
JS

読みやすいコードに唯一の正解はありません。そこで、好みではなく「次に変更する人が迷う場所」を探します。

練習前のコード

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);

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

参考リソース

次に読む記事

おすすめ記事

← 一覧に戻る
PR
PR
PR
PR