2026-05-17 弱点補強 | CQS × pytest設計

概要

⚡

コマンド・クエリ分離（CQS）

状態を変えるメソッドは値を返さない。値を返すメソッドは状態を変えない。

🚩

フラグ引数廃止

bool フラグを持つメソッドは「実は複数メソッドに分けるべき」サイン。

🚫

null 返却禁止

Optional[T] の代わりに例外を使い、呼び出し元の None チェックを排除する。

🧊

不変オブジェクト

frozen=True で値オブジェクトを不変にし、予期しない変更を防ぐ。

問題

以下の悪いコードを読んで問題点を指摘し、第13章（メソッド設計）の原則を適用してリファクタリングしてください。

問題の背景

項目	内容
コンテキスト	MOps（販促）チームの施策ウェブフックから呼び出されているカートサービス
関係者	複数の販促バッチからもクーポンロジックを参照している
課題	pytestでの自動テストを追加しようとしているが、テストが書きにくい

制約・前提条件

Python 3.12+（dataclass, match文, StrEnumが使える）
外部APIや DB は呼ばず、ピュアなドメインロジックとして実装
不変（frozen=True）を使えるところは使うこと
pytestで3ケース以上のテストコードも合わせて書くこと

期待する回答形式: コード（改善後）＋ pytestコード＋問題点の列挙＋設計ポイント

悪いコード (Before)

このコードには 6つの設計上の問題 が隠れています。見つけてみてください。

# cart.py — ECサイト カートサービス（MOps文脈）
from dataclasses import dataclass, field
from typing import Optional
import logging

logger = logging.getLogger(__name__)

@dataclass
class CartItem:
    sku: str
    price: float          # ← 問題: float で金額計算
    qty: int

@dataclass
class Cart:
    user_id: str
    items: list[CartItem] = field(default_factory=list)
    coupon_applied: bool = False

    def update_item(self, sku: str, qty: int,
                    apply_coupon: bool = False,   # ← 問題: フラグ引数
                    remove: bool = False) -> Optional[float]:  # ← 問題: null返却
        """カートアイテムを更新し、適用後の合計を返す。"""
        # ← 問題: コマンド（remove）とクエリ（合計返却）が混在
        if remove:
            self.items = [i for i in self.items if i.sku != sku]
            logger.info(f"removed {sku}")
            return None           # ← None返却
        for item in self.items:
            if item.sku == sku:
                item.qty = qty    # ← CartItem が mutable
                if apply_coupon:
                    self.coupon_applied = True
                total = sum(i.price * i.qty for i in self.items)
                if self.coupon_applied:
                    total *= 0.9
                return total      # ← 状態変更しながら値も返す (CQS違反)
        return None               # ← None返却

    def get_total(self) -> float:
        total = sum(i.price * i.qty for i in self.items)
        if self.coupon_applied:
            self.coupon_applied = False  # ← 問題: クエリで副作用（クーポンをリセット）
            total *= 0.9
        return total

ヒント（段階的開示）

ヒント1 — 方向性

「コマンド（状態を変える処理）」と「クエリ（値を返す処理）」が1つのメソッドに混在しています。それぞれを別のメソッドに分離することを考えてください。

ヒント2 — アプローチ（3カテゴリ整理）

問題点を3つのカテゴリで整理してみましょう：

CQS違反 update_item() は状態変更＋合計値返却を同時にやっている。get_total() は値を返しながら状態を変えている
フラグ引数 apply_coupon: bool, remove: bool という複数フラグは「実は複数のメソッドに分けるべき」サイン
null返却 Optional[float] を返すことでテスト側が常に None チェックを強いられる

ヒント3 — 目指す構造

✗ Before（混在）

def update_item(self, sku, qty,
    apply_coupon=False,
    remove=False) -> Optional[float]:
    # 状態変更 + 値返却 が混在
    ...

✓ After（分離）

# コマンド（状態変更のみ、戻り値なし）
def set_quantity(self, sku, qty) -> None
def remove_item(self, sku) -> None
def apply_coupon(self) -> None

# クエリ（状態変更なし、値返却のみ）
@property
def total(self) -> Decimal
def discounted_total(self, rate) -> Decimal

None を返す代わりに SkuNotFoundError を送出。 float ではなく Decimal を使うことで金額計算の精度問題を防げます。

問題点分析

#	問題点	分類	影響	改善方法
1	`update_item()` が状態変更＋値返却を同時に行う	CQS違反	テストで「副作用の有無」を独立して検証できない	`set_quantity()` と `total` プロパティに分離
2	`get_total()` が `coupon_applied` をリセットする副作用を持つ	CQS違反	合計を2回参照するだけでクーポンが消滅するバグ	クエリを副作用フリーのプロパティに変更
3	`apply_coupon: bool`, `remove: bool` のフラグ引数	フラグ引数	呼び出しコードが `update_item("A", 1, False, True)` になり読めない	`apply_coupon()` / `remove_item()` に分割
4	`Optional[float]` を返す（None返却）	null返却	呼び出し元が毎回 `if result is None` を書く必要あり	`SkuNotFoundError` を送出して例外で制御
5	金額を `float` で扱っている	精度問題	`0.1 + 0.2 != 0.3` 問題が金額計算で発生しうる	`Decimal` に変更（ECサイト実務必須）
6	`CartItem` が mutable（`item.qty = qty` で直接変更）	不変性	共有参照時に予期しない変更が起きる。テストの分離が困難	`frozen=True` ＋ `with_qty()` で新インスタンス生成

CQS 構造図 — リファクタリング後の設計

コマンド（状態変更）とクエリ（値取得）を完全に分離した構造。

コマンド（左）は Cart の状態を変更するが値を返さない。クエリ（右）は値を返すが Cart の状態を一切変えない。この分離がテストの書きやすさと副作用バグの防止を実現する。

模範解答

# cart.py — ECサイト カートサービス（リファクタリング後）
from __future__ import annotations

from dataclasses import dataclass, field
from decimal import Decimal
from typing import Final

DEFAULT_COUPON_DISCOUNT_RATE: Final[Decimal] = Decimal("0.10")


class SkuNotFoundError(KeyError):
    """カートに存在しないSKUを操作しようとした場合に発生する例外。"""


@dataclass(frozen=True)          # ← 不変オブジェクト
class CartItem:
    sku: str
    unit_price: Decimal          # ← float → Decimal で精度保証
    qty: int

    def __post_init__(self) -> None:
        if self.qty <= 0:
            raise ValueError(f"qty は1以上でなければなりません: qty={self.qty}")

    @property
    def subtotal(self) -> Decimal:
        """クエリ: 状態変更なし"""
        return self.unit_price * self.qty

    def with_qty(self, qty: int) -> CartItem:
        """不変パターン: 数量変更は新インスタンスを返す"""
        return CartItem(sku=self.sku, unit_price=self.unit_price, qty=qty)


@dataclass
class Cart:
    user_id: str
    _items: dict[str, CartItem] = field(default_factory=dict, init=False, repr=False)
    _coupon_applied: bool = field(default=False, init=False, repr=False)

    # ── Commands: 状態変更のみ、return None ───────────────────────────────

    def add_item(self, item: CartItem) -> None:
        if item.sku in self._items:
            existing = self._items[item.sku]
            self._items[item.sku] = existing.with_qty(existing.qty + item.qty)
        else:
            self._items[item.sku] = item

    def set_quantity(self, sku: str, qty: int) -> None:
        self._require_sku(sku)
        self._items[sku] = self._items[sku].with_qty(qty)

    def remove_item(self, sku: str) -> None:
        self._require_sku(sku)
        del self._items[sku]

    def apply_coupon(self) -> None:
        """冪等: 2回呼んでも状態は変わらない"""
        self._coupon_applied = True

    # ── Queries: 副作用なし、値を返す ────────────────────────────────────

    @property
    def raw_total(self) -> Decimal:
        return sum((item.subtotal for item in self._items.values()), Decimal("0"))

    @property
    def total(self) -> Decimal:
        if self._coupon_applied:
            return self.discounted_total(DEFAULT_COUPON_DISCOUNT_RATE)
        return self.raw_total

    def discounted_total(self, discount_rate: Decimal) -> Decimal:
        if not (Decimal("0") <= discount_rate < Decimal("1")):
            raise ValueError(f"discount_rate は [0, 1) の範囲で: {discount_rate}")
        return self.raw_total * (Decimal("1") - discount_rate)

    @property
    def item_count(self) -> int:
        return len(self._items)

    # ── Private ──────────────────────────────────────────────────────────

    def _require_sku(self, sku: str) -> None:
        if sku not in self._items:
            raise SkuNotFoundError(f"SKU '{sku}' はカートに存在しません")

# test_cart.py
from decimal import Decimal
import pytest
from cart import Cart, CartItem, SkuNotFoundError


@pytest.fixture
def cart() -> Cart:
    return Cart(user_id="u001")

@pytest.fixture
def item_a() -> CartItem:
    return CartItem(sku="A001", unit_price=Decimal("1000"), qty=2)  # subtotal=2000

@pytest.fixture
def item_b() -> CartItem:
    return CartItem(sku="B001", unit_price=Decimal("500"), qty=3)   # subtotal=1500


class TestRawTotal:
    """クエリの副作用なしを検証"""

    def test_empty_cart_returns_zero(self, cart):
        assert cart.raw_total == Decimal("0")

    def test_single_item_subtotal(self, cart, item_a):
        cart.add_item(item_a)
        assert cart.raw_total == Decimal("2000")

    def test_multiple_items_summed(self, cart, item_a, item_b):
        cart.add_item(item_a)
        cart.add_item(item_b)
        assert cart.raw_total == Decimal("3500")


class TestCoupon:
    """CQS: クエリがクーポン状態をリセットしないことを検証"""

    def test_apply_coupon_reduces_total_by_10_percent(self, cart, item_a):
        cart.add_item(item_a)
        cart.apply_coupon()
        assert cart.total == Decimal("1800")   # 2000 * 0.90

    def test_get_total_does_NOT_reset_coupon(self, cart, item_a):
        """Before の get_total() は2回目でクーポンが消えた — After では消えない"""
        cart.add_item(item_a)
        cart.apply_coupon()
        first  = cart.total
        second = cart.total
        assert first == second == Decimal("1800")   # ← 副作用なしを証明

    def test_apply_coupon_is_idempotent(self, cart, item_a):
        cart.add_item(item_a)
        cart.apply_coupon()
        cart.apply_coupon()   # 2回目は変化なし
        assert cart.total == Decimal("1800")


class TestRemoveItem:
    """コマンド: 戻り値なし、存在しないSKUは例外"""

    def test_remove_existing_item(self, cart, item_a, item_b):
        cart.add_item(item_a)
        cart.add_item(item_b)
        cart.remove_item("A001")
        assert cart.item_count == 1
        assert cart.raw_total == Decimal("1500")

    def test_remove_nonexistent_sku_raises(self, cart):
        with pytest.raises(SkuNotFoundError):
            cart.remove_item("GHOST")


class TestSetQuantity:
    """フラグ引数廃止 → 専用コマンドの検証"""

    def test_set_quantity_updates_subtotal(self, cart, item_a):
        cart.add_item(item_a)          # qty=2, subtotal=2000
        cart.set_quantity("A001", 5)   # qty=5, subtotal=5000
        assert cart.raw_total == Decimal("5000")

    def test_set_quantity_invalid_qty_raises(self, cart, item_a):
        cart.add_item(item_a)
        with pytest.raises(ValueError):
            cart.set_quantity("A001", 0)


class TestDiscountedTotal:
    """クエリの再利用性を検証"""

    def test_custom_20_percent_discount(self, cart, item_a):
        cart.add_item(item_a)
        assert cart.discounted_total(Decimal("0.20")) == Decimal("1600")

    def test_invalid_rate_raises(self, cart, item_a):
        cart.add_item(item_a)
        with pytest.raises(ValueError):
            cart.discounted_total(Decimal("1.00"))

ポイント解説

1 コマンド・クエリ分離（CQS原則 — Ch13）
update_item() という「万能メソッド」を、コマンド群（add_item / set_quantity / remove_item）とクエリ群（raw_total / total / discounted_total）に分離。コマンドは None を返し、クエリは状態を変えない。

2 フラグ引数廃止（Ch13）
apply_coupon: bool, remove: bool を持つメソッドは実質3つの処理を1つに押し込んでいた。それぞれ apply_coupon() / remove_item() という明示的メソッドに分けることで、呼び出しコードが update_item("A", 1, False, True) → remove_item("A") と読めるようになる。

3 null返却禁止（Ch13）
Optional[float] を返す代わりに SkuNotFoundError を送出。呼び出し元が if result is None を書く必要がなくなり、テストも「例外が発生するか否か」で明確に書けるようになる。

4 副作用のないクエリでテストが書きやすくなる
Before では get_total() を呼ぶたびに coupon_applied がリセットされるため、「合計を2回参照する」テストが壊れた。 After では total は純粋クエリなので何回呼んでも同じ結果が返る。 test_get_total_does_NOT_reset_coupon がこれを証明している。

5 float → Decimal で精度保証
0.1 + 0.2 != 0.3 問題を防ぐ。ECサイトでは金額に Decimal が必須。 Decimal("0.10") と文字列リテラルから生成することで精度が保証される。

6 不変 CartItem（frozen=True）
アイテムの変更は with_qty() で新インスタンスを生成する不変パターン。 CartItem を共有参照した場合でも予期しない変更が起きない。

Before vs After 比較

観点	Before	After
メソッド数	2（update_item, get_total）	8（コマンド4 + クエリ4）
テスト容易性	副作用が絡み合い難しい	コマンド/クエリ独立でテスト可能
None返却	`Optional[float]` あり	例外のみ（Noneなし）
フラグ引数	2個（apply_coupon, remove）	0個
金額精度	`float`（誤差あり）	`Decimal`（精度保証）
CartItem	mutable（直接変更可）	frozen=True（不変）

実務への応用

MOps/販促システムでの実際の場面

バッチがクーポンを消す副作用バグ: 販促バッチが get_total() を呼び出した際に、その副作用（クーポンリセット）によって「バッチが1回実行されるとクーポンが消える」バグが発生しうる。クエリを副作用フリーにすることで構造的に防げる。
複数施策の共存: 10%OFF・20%OFF・ポイント還元が存在する場合、discounted_total(rate) にパラメータを渡す設計にすることで施策ごとにコードを書かずに済む。
Argo Workflowsのリトライ制御: SkuNotFoundError を明示的に発生させることで、 retryStrategy で「SkuNotFoundErrorはリトライしない」制御が自然に書ける。

次のステップ

発展問題: Cart に「販促ルール」（例: 3000円以上で送料無料、特定カテゴリ10%OFF）を追加する際に、 ストラテジパターン（Ch8） を使って DiscountPolicy として分離してください。

参考: 「良いコード・悪いコードで学ぶ設計入門（改訂新版）」Ch13 メソッド設計
参考: PEP 526（Decimal in financial applications）
参考: pytest fixtures公式ドキュメント

今日のまとめ

コマンド・クエリ分離（CQS）は「状態を変えるメソッドは値を返さない・値を返すメソッドは状態を変えない」という原則で、テストが書きやすくなり副作用バグを構造的に防げる。

フラグ引数を廃止してメソッドを分けること・null返却の代わりに例外を使うことが、呼び出し元コードの品質を直接高める。

ECサイトでの金額計算は必ず Decimal を使うこと（float は誤差が出る）。

自己評価

完全理解おおむね理解要復習

弱点補強 — コマンド・クエリ分離（CQS）× pytest設計

概要

コマンド・クエリ分離（CQS）

フラグ引数廃止

null 返却禁止

不変オブジェクト

問題

問題の背景

制約・前提条件

悪いコード (Before)

ヒント（段階的開示）

問題点分析

CQS 構造図 — リファクタリング後の設計

模範解答

ポイント解説

Before vs After 比較

実務への応用

次のステップ

今日のまとめ

自己評価

自分の回答

気づき・メモ