概要
仕様化テスト Characterization Test(Ch15)
構造を変える前に、代表的な入力パターンで今の実装が実際に返す値をそのままテストとして固定する。バグに見える挙動も含め、それが「今の仕様」として一旦守られる対象になる。
スモールステップ(Ch15)
1ステップ=1関心事のみを変更し、そのたびにテストを実行する。テストが赤くなったら疑うべき範囲は直前の1ステップだけに絞られ、原因特定のコストが下がる。
IDE活用(Ch15)
Extract Method / Rename Symbol のような自動リファクタリングを使うことで、手作業のコピペで発生する「条件の消し忘れ」を機械的に防ぐ。
実際に起きた事故
テストなしで構造を先に変えたところ、NEWBIE500クーポンの「初回購入者のみ」という条件が分割の過程で抜け落ち、既存顧客にも割引が適用される事故が本番で発生した。
問題
EC企業の販促(MOps)システムには、注文金額にクーポン・会員ランク・キャンペーンの割引を順に適用する calc_final_price が本番稼働している。この関数にはユニットテストが1つも存在せず、仕様書も散逸している。
NEWBIE500 クーポンの「初回購入者(order_count == 1)のみ適用」という条件チェックが分割の過程で抜け落ち、2回目以降の購入者にも500円引きが適用されるバグが本番で発生した。レビューでも「テストが通っている」という機械的な安心材料がなく、目視レビューだけでは気づけなかった。
「良いコード・悪いコードで学ぶ設計入門」第15章(リファクタリング)の考え方——仕様化テスト(Characterization Test)で現状の振る舞いを先に固定し、スモールステップで安全にリファクタリングする——に従い、この事故を再発させない進め方で calc_final_price を整理してください。
制約・前提条件
- Python 3.12+ / pytest
- 既存の振る舞い(
NEWBIE500が初回購入者にしか適用されない、という現状の仕様も含む)は、今回のリファクタリングでは変更しない。挙動を変えたい場合は別途チケットを起票し、今回とは別のステップで対応する方針とする - 仕様化テストは pytest の
parametrizeを使い、クーポンなし/SUMMER10(閾値未満・以上)/NEWBIE500(初回・2回目以降)/会員ランク(none/gold/platinum)/キャンペーン有無/価格がマイナスになる境界値、を含む8ケース以上で現状の出力を固定すること - リファクタリングはIDEの「メソッドの抽出(Extract Method)」に相当する操作で、1関心事ずつ小さく進めること(一度に複数の変更を混ぜない)
- マジックナンバー(
5000,0.1,300,500,0.95,0.9,10000,1000)は名前付き定数にすること - 最終コードには Google スタイル docstring を付け、割引の適用順序(クーポン→会員ランク→キャンペーン→0円クランプ)を明記すること
悪い進め方 (Before)
def calc_final_price(order_total, coupon_code, member_rank, campaign_active, order_count):
price = order_total
if coupon_code == "SUMMER10":
if order_total >= 5000:
price = price - price * 0.1
else:
price = price - 300
elif coupon_code == "NEWBIE500":
if order_count == 1:
price = price - 500
if member_rank == "gold":
price = price * 0.95
elif member_rank == "platinum":
price = price * 0.9
if campaign_active:
if price > 10000:
price = price - 1000
if price < 0:
price = 0
return round(price)
# 問題: このコードにはユニットテストが1つもない。
# 担当者はこの状態のまま「先にネスト解消・関数分割をして、
# 後でテストを書く」方針でリファクタリングに着手した。
order_count == 1(初回限定)という条件は自明ではなく、分割の過程で脈絡なく消えても気づきにくいヒント(段階的開示)
ヒント1 — 方向性
order_count == 1 のような一見地味な条件は見落とされやすい。第15章のリファクタリングは「構造を変える」作業と「動作を変える」作業を明確に分け、前者を行う前に必ず後者が起きていないことを検証できる状態を作ることを説く。つまり、最初にやるべきことはコードを触ることではなく、「今のコードが何を返すか」を網羅的に記録して固定することである。
ヒント2 — アプローチ
- まず
calc_final_priceに対して、代表的な入力パターン(クーポンなし/SUMMER10の2条件/NEWBIE500の2条件/会員ランク3種/キャンペーン有無/マイナスになる境界値)をpytest.mark.parametrizeで洗い出し、現在の実装が実際に返す値をそのまま期待値として書く(バグに見える挙動も含め、それが「今の仕様」として一旦固定される) - このテストが全てパスすることを確認したら、初めてリファクタリングに着手する。1回の変更で複数の関心事(クーポン判定・会員ランク判定・キャンペーン判定・クランプ処理)をまとめて分割しない
- 1つの関心事を関数として抽出する → その場でテストを実行し、全件グリーンであることを確認する → 次の関心事へ進む、というサイクルを繰り返す。テストが1件でも落ちたら、直前の変更だけを疑えばよい(変更範囲が小さいので原因特定が速い)
- IDEの自動リファクタリング機能(Extract Method / Rename Symbol)を使うと、手作業でのコピペミス(今回の事故のような条件の消し忘れ)を避けやすい
ヒント3 — コードの骨格
# Step0: 仕様化テスト(このテストが全ステップを通じてグリーンであり続けることを担保にする)
import pytest
@pytest.mark.parametrize(
"order_total, coupon_code, member_rank, campaign_active, order_count, expected",
[
(3000, None, None, False, 3, 3000),
(4000, "SUMMER10", None, False, 3, 3700),
(6000, "SUMMER10", None, False, 3, 5400),
(3000, "NEWBIE500", None, False, 1, 2500),
(3000, "NEWBIE500", None, False, 2, 3000), # ← ここが事故で壊れた挙動
# ... 会員ランク・キャンペーン・境界値ケースを追加
],
)
def test_calc_final_price_characterization(...):
assert calc_final_price(...) == expected
# Step1〜: 1関心事ずつ抽出。骨格のみ提示
def _apply_coupon(price, order_total, coupon_code, order_count): ...
def _apply_member_rank_discount(price, member_rank): ...
def _apply_campaign_discount(price, campaign_active): ...
def _clamp_non_negative(price): ...
def calc_final_price(order_total, coupon_code, member_rank, campaign_active, order_count):
price = order_total
price = _apply_coupon(price, order_total, coupon_code, order_count)
price = _apply_member_rank_discount(price, member_rank)
price = _apply_campaign_discount(price, campaign_active)
price = _clamp_non_negative(price)
return round(price)
問題点分析(5点)
| # | 問題点 | 分類 | 改善方法 |
|---|---|---|---|
| 1 | テストなしで構造を先に変更 | 安全網の欠如 Ch15 | 仕様化テストを最初に書く |
| 2 | 「後でテストを書く」の順序が逆 | 検証順序の誤り Ch15 | 変更前にテストで現状を固定 |
| 3 | 複数の関心事を同時に変更 | 原因特定コスト増 Ch15 | 1ステップ=1関心事に分解 |
| 4 | 初回限定クーポンの暗黙仕様が消失 | 暗黙知の喪失 | テストケースとして明示 |
| 5 | 目視レビューのみに依存 | 検証手段の欠如 | テストのグリーンを機械的な根拠にする |
安全網の作り方(SVG)— Bad な進め方 vs Good なスモールステップ
模範解答
仕様化テスト(Step0)
# tests/test_calc_final_price_characterization.py
"""calc_final_price のリファクタリング前に、現状の振る舞いを固定するテスト。
良いコード・悪いコードで学ぶ設計入門(改訂新版)
Ch15: リファクタリング — 仕様化テスト(Characterization Test)
このテストは「今のコードが何を返すか」をそのまま記録したものであり、
NEWBIE500の初回限定条件のようなバグに見える挙動も含めて、リファクタリング
完了までは仕様として固定する。挙動を変えたい場合は別チケットで対応する。
"""
import pytest
from promotion.discount import calc_final_price # リファクタリング対象
@pytest.mark.parametrize(
"order_total, coupon_code, member_rank, campaign_active, order_count, expected",
[
# クーポンなし・ランクなし・キャンペーンなし
(3000, None, None, False, 3, 3000),
# SUMMER10: 閾値(5000円)未満 -> 定額300円引き
(4000, "SUMMER10", None, False, 3, 3700),
# SUMMER10: 閾値(5000円)以上 -> 10%引き
(6000, "SUMMER10", None, False, 3, 5400),
# NEWBIE500: 初回購入(order_count == 1)のみ適用
(3000, "NEWBIE500", None, False, 1, 2500),
# NEWBIE500: 2回目以降は適用されない
# ← この行が「本番事故」で壊れた挙動そのもの。
# リファクタリング中にこのケースが赤くなったら即座に原因を疑う。
(3000, "NEWBIE500", None, False, 2, 3000),
# 会員ランクgold: 5%引き
(4000, None, "gold", False, 3, 3800),
# 会員ランクplatinum: 10%引き
(4000, None, "platinum", False, 3, 3600),
# キャンペーン中かつ10000円超で1000円引き
(12000, None, None, True, 3, 11000),
# クーポン+ランク+キャンペーンの重複適用(適用順序を固定する)
(12000, "SUMMER10", "gold", True, 3, 9260),
# 割引の重複適用で0円未満になる境界値 -> 0円にクランプ
(200, "SUMMER10", None, False, 3, 0),
],
)
def test_calc_final_price_characterization(
order_total, coupon_code, member_rank, campaign_active, order_count, expected
):
"""既存実装の現状の挙動をそのまま固定するテスト。
このテストが全ステップを通じてグリーンであり続けることを、
「構造だけを変え、振る舞いは変えていない」ことの担保とする。
"""
assert (
calc_final_price(order_total, coupon_code, member_rank, campaign_active, order_count)
== expected
)
スモールステップでのリファクタリング手順
calc_final_price に対して実行して全件グリーンであることを確認する。ここまではコードを一切変更しない。coupon_code に関するif/elifブロックを、IDEの「メソッドの抽出」機能で _apply_coupon(price, order_total, coupon_code, order_count) として切り出す。切り出した直後にテストを再実行し、10件全てグリーンであることを確認してから次へ進む。ここで order_count == 1 の条件を消さずにそのまま移植できているかがテストで機械的に検証される。member_rank に関するif/elifブロックを _apply_member_rank_discount(price, member_rank) として抽出。テスト実行 → 全件グリーンを確認。campaign_active のネストしたifブロックを _apply_campaign_discount(price, campaign_active) として抽出。テスト実行 → 全件グリーンを確認。if price < 0: price = 0 を _clamp_non_negative(price) として抽出。テスト実行 → 全件グリーンを確認。5000, 0.1, 300, 500, 0.95, 0.9, 10000, 1000 を名前付き定数に置き換える。置き換え後にテストを実行し、全件グリーンを確認する。改善後コード
"""discount.py — クーポン・会員ランク・キャンペーンによる注文金額の割引計算。
良いコード・悪いコードで学ぶ設計入門(改訂新版)
Ch15: リファクタリング — 仕様化テストとスモールステップによる安全な構造変更
このモジュールは以下の手順で安全にリファクタリングされた:
Step0: 仕様化テストで現状の挙動を固定(tests/test_calc_final_price_characterization.py)
Step1-4: クーポン/会員ランク/キャンペーン/クランプの各関心事を
IDEの「メソッドの抽出」機能で1つずつ抽出し、都度テストがグリーンであることを確認
Step5: マジックナンバーを名前付き定数化
Step6: docstring整備
"""
from __future__ import annotations
from typing import Final
# --- 定数(マジックナンバー排除) ---
SUMMER10_THRESHOLD_YEN: Final[int] = 5000
SUMMER10_RATE: Final[float] = 0.1
SUMMER10_FLAT_DISCOUNT_YEN: Final[int] = 300
NEWBIE_COUPON_DISCOUNT_YEN: Final[int] = 500
NEWBIE_TARGET_ORDER_COUNT: Final[int] = 1
GOLD_RANK_RATE: Final[float] = 0.95
PLATINUM_RANK_RATE: Final[float] = 0.9
CAMPAIGN_THRESHOLD_YEN: Final[int] = 10000
CAMPAIGN_FLAT_DISCOUNT_YEN: Final[int] = 1000
def _apply_coupon(
price: float, order_total: int, coupon_code: str | None, order_count: int
) -> float:
"""クーポン種別に応じた割引を適用する。
Args:
price: 現時点での価格。
order_total: クーポン適用条件(閾値判定)に使う元の注文金額。
coupon_code: 適用するクーポンコード(未使用時はNone)。
order_count: 会員の累計購入回数(初回限定クーポンの判定に使用)。
Returns:
クーポン適用後の価格。
"""
if coupon_code == "SUMMER10":
if order_total >= SUMMER10_THRESHOLD_YEN:
return price - price * SUMMER10_RATE
return price - SUMMER10_FLAT_DISCOUNT_YEN
if coupon_code == "NEWBIE500":
# 初回購入(order_count == 1)のみ適用。過去にこの分岐が
# リファクタリング中に抜け落ち、既存顧客にも適用される事故が発生した箇所。
if order_count == NEWBIE_TARGET_ORDER_COUNT:
return price - NEWBIE_COUPON_DISCOUNT_YEN
return price
return price
def _apply_member_rank_discount(price: float, member_rank: str | None) -> float:
"""会員ランクに応じた率引きを適用する。
Args:
price: 現時点での価格。
member_rank: 会員ランク("gold" / "platinum" / None)。
Returns:
会員ランク割引適用後の価格。
"""
if member_rank == "gold":
return price * GOLD_RANK_RATE
if member_rank == "platinum":
return price * PLATINUM_RANK_RATE
return price
def _apply_campaign_discount(price: float, campaign_active: bool) -> float:
"""キャンペーン期間中、一定額以上の注文に定額割引を適用する。
Args:
price: 現時点での価格。
campaign_active: キャンペーン開催中かどうか。
Returns:
キャンペーン割引適用後の価格。
"""
if campaign_active and price > CAMPAIGN_THRESHOLD_YEN:
return price - CAMPAIGN_FLAT_DISCOUNT_YEN
return price
def _clamp_non_negative(price: float) -> float:
"""割引の重複適用で金額がマイナスになった場合に0円へ丸める。
Args:
price: クランプ前の価格。
Returns:
0円未満にならないようにクランプした価格。
"""
return max(price, 0)
def calc_final_price(
order_total: int,
coupon_code: str | None,
member_rank: str | None,
campaign_active: bool,
order_count: int,
) -> int:
"""注文金額に対してクーポン・会員ランク・キャンペーン割引を順に適用する。
適用順序(既存仕様として固定。変更する場合は別途PMと合意の上で
仕様化テストを更新すること):
1. クーポン割引 (_apply_coupon)
2. 会員ランク割引 (_apply_member_rank_discount)
3. キャンペーン割引 (_apply_campaign_discount)
4. 0円未満のクランプ (_clamp_non_negative)
Args:
order_total: 割引前の注文金額(円)。
coupon_code: 適用するクーポンコード。未指定はNone。
member_rank: 会員ランク("gold" / "platinum" / None)。
campaign_active: キャンペーン開催中かどうか。
order_count: 会員の累計購入回数。
Returns:
割引適用後の最終金額(円、整数に丸め)。
Examples:
>>> calc_final_price(12000, "SUMMER10", "gold", True, 3)
9260
"""
price: float = order_total
price = _apply_coupon(price, order_total, coupon_code, order_count)
price = _apply_member_rank_discount(price, member_rank)
price = _apply_campaign_discount(price, campaign_active)
price = _clamp_non_negative(price)
return round(price)
# クーポンなし・ランクなし・キャンペーンなし
calc_final_price(3000, None, None, False, 3)
# 3000
# SUMMER10(閾値未満): 定額300円引き
calc_final_price(4000, "SUMMER10", None, False, 3)
# 3700
# NEWBIE500: 初回購入のみ500円引き(2回目以降は適用されない = 事故を防いだ挙動)
calc_final_price(3000, "NEWBIE500", None, False, 1)
# 2500
calc_final_price(3000, "NEWBIE500", None, False, 2)
# 3000
# クーポン + 会員ランク + キャンペーンの重複適用
calc_final_price(12000, "SUMMER10", "gold", True, 3)
# 9260 (12000 -> 10800 [SUMMER10 10%] -> 10260 [gold 5%] -> 9260 [キャンペーン1000円引き])
# 割引の重複適用で0円未満 -> クランプ
calc_final_price(200, "SUMMER10", None, False, 3)
# 0
| ポイント | 適用した設計原則 | 書籍対応章 |
|---|---|---|
| 構造変更の前に振る舞いを固定するテストを書く | 仕様化テスト(Characterization Test) | Ch15 |
| 1関心事ずつ抽出し、都度テストを実行して確認する | スモールステップでのリファクタリング | Ch15 |
| 手作業のコピペではなくIDEの自動リファクタリングを使う | IDE活用(Extract Method / Rename Symbol) | Ch15 |
_apply_coupon / _apply_member_rank_discount / _apply_campaign_discount / _clamp_non_negative への分割 | 関心の分離・単一責任 | Ch6-7 |
5000/0.1/300/500/0.95/0.9/10000/1000 → 名前付き定数 | マジックナンバーの排除 | Ch10 |
このページ生成時に、元の calc_final_price(Before)とリファクタリング後の実装(After)の両方に対して同一の10ケースを実行し、全て一致することを確認済み。
[3000, None, None, False, 3] -> 3000 expected 3000 OK
[4000, 'SUMMER10', None, False, 3] -> 3700 expected 3700 OK
[6000, 'SUMMER10', None, False, 3] -> 5400 expected 5400 OK
[3000, 'NEWBIE500', None, False, 1] -> 2500 expected 2500 OK
[3000, 'NEWBIE500', None, False, 2] -> 3000 expected 3000 OK
[4000, None, 'gold', False, 3] -> 3800 expected 3800 OK
[4000, None, 'platinum', False, 3] -> 3600 expected 3600 OK
[12000, None, None, True, 3] -> 11000 expected 11000 OK
[12000, 'SUMMER10', 'gold', True, 3] -> 9260 expected 9260 OK
[200, 'SUMMER10', None, False, 3] -> 0 expected 0 OK
ALL OK — Before と After は完全に一致(振る舞いは変わっていない)
ポイント解説
NEWBIE500 が2回目以降の購入者に適用されない、という挙動が意図した仕様なのかバグなのかは、リファクタリングの時点では判断しない。まず現状を固定し、挙動を変えたいならそれは別のタスク(別チケット)として扱う。この区別を怠ると、「リファクタリングのつもりが仕様変更も混ざっていた」という状態になり、原因特定が難しくなる。NEWBIE500 のバグがどの変更で混入したのかを特定するのに、変更全体を洗い直す必要があった。order_count == 1 の消失のような事故は、手作業でのコピペより自動リファクタリングの方が起きにくい。実務への応用
MOpsの割引計算のように「金額を直接左右し、かつ条件分岐が複雑化しやすい」関数は、事業上のインパクトが大きい一方でテストが後回しにされがちな典型例である。今回のような「テストなしリファクタリングで本番事故」というパターンは、割引計算に限らず、送料計算・在庫引き当てロジック・ポイント付与ロジックなど、MOps/販促システム全体で繰り返し起こりうる。
今日のまとめ
次のステップ
- 発展問題: クーポン種別が
SUMMER10/NEWBIE500の2種類から、将来的に「複数クーポンの併用可否ルール」を持つN種類に拡張される場合を想定し、_apply_couponの if/elif連鎖がどのように破綻するか(新種別追加のたびに関数が肥大化する)を検討し、第8章(条件分岐)のストラテジパターンやポリシーパターンでどう置き換えられるかを設計してください。 - 参考: 「良いコード・悪いコードで学ぶ設計入門(改訂新版)」Ch15(リファクタリング)/ Ch8(条件分岐・ストラテジパターン)/ Martin Fowler "Refactoring"(Characterization Test の元になった概念)