A コーディング — 意味不明な命名・深いネスト・データクラス問題（キャンペーン割引 CampaignDiscountData Bad→Good Ch1-2）

概要

問題

EC企業 MOps 販促システムチームでは、注文にキャンペーン割引を適用する処理が3箇所に分散して実装されている。

• Mgr（注文確定時の最終金額計算）
• CartPreviewService（カート画面のプレビュー金額計算）
• NotificationService（Slackへの割引適用通知）

「良いコード・悪いコードで学ぶ設計入門」第1-2章（悪しき構造・設計の初歩：意味不明な命名、ネスト、データクラス問題）の考え方を使って、根本原因を構造の問題として説明した上でリファクタリングしてください。

制約・前提条件

• Python 3.12+ を前提とする（StrEnum, match文, dataclass(slots=True) などモダン記法を使ってよい）
• CampaignDiscountData は公開フィールドのみを持つデータの入れ物であり、「対象かどうか」「いくら割引するか」の判断ロジックが3箇所に重複実装されている、という構造上の欠陥を名指しで指摘すること
• リファクタリング後は、今回のバグ（CartPreviewServiceだけend_atチェックが漏れる）が構造的に再発しえないことを説明すること

悪いコード (Before)

📋 コピーfrom dataclasses import dataclass
from datetime import datetime
from decimal import Decimal


@dataclass
class CampaignDiscountData:
    id: str
    type: str  # "PERCENT" or "FIXED"
    value: Decimal
    start_at: datetime
    end_at: datetime
    target_rank: str  # "ALL" / "GOLD" / "PLATINUM"
    is_active: bool


class Mgr:
    """キャンペーン割引を注文に適用する。"""

    def calc(self, o, c, d):
        # o: order, c: customer, d: CampaignDiscountData
        t = Decimal("0")
        if d is not None:
            if d.is_active:
                if d.end_at > datetime.now():
                    if d.target_rank == "ALL" or d.target_rank == c.rank:
                        if d.type == "PERCENT":
                            t = o.subtotal * (d.value / Decimal("100"))
                        else:
                            if d.type == "FIXED":
                                t = d.value
        return o.subtotal - t


class CartPreviewService:
    """カート画面のプレビュー金額を計算する。"""

    def preview(self, o, c, d):
        r = Decimal("0")
        if d is not None:
            if d.is_active:
                if d.target_rank == "ALL" or d.target_rank == c.rank:
                    if d.type == "PERCENT":
                        r = o.subtotal * (d.value / Decimal("100"))
                    else:
                        if d.type == "FIXED":
                            r = d.value
        # NOTE: end_at のチェックが Mgr.calc と食い違っている
        return o.subtotal - r


class NotificationService:
    """Slackへの割引適用通知を送る。"""

    def build_message(self, o, c, d):
        f = Decimal("0")
        if d is not None:
            if d.is_active:
                if d.end_at > datetime.now():
                    if d.target_rank == "ALL" or d.target_rank == c.rank:
                        if d.type == "PERCENT":
                            f = o.subtotal * (d.value / Decimal("100"))
                        else:
                            if d.type == "FIXED":
                                f = d.value
        return f"割引 {f}円 を適用しました（注文ID: {o.id}）"

ヒント（段階的開示）

class CampaignDiscount:  # ← Data サフィックスを外し、振る舞いを持つドメインオブジェクトへ
    def is_applicable_to(self, customer_rank, at) -> bool:
        if not self.is_active:
            return False
        if not (self.start_at <= at <= self.end_at):
            return False
        return self.target_rank == CustomerRank.ALL or self.target_rank == customer_rank

    def discount_amount(self, subtotal) -> Decimal:
        match self.discount_type:
            case DiscountType.PERCENT: ...
            case DiscountType.FIXED: ...

問題点分析（7点）

構造の比較（SVG）— 判定ロジックの漏れ出し vs ドメインオブジェクトへの集約

模範解答

📋 コピー"""キャンペーン割引適用モジュール。

良いコード・悪いコードで学ぶ設計入門（改訂新版）
  第1-2章: 悪しき構造・設計の初歩 — 意味不明な命名、ネスト、データクラス問題

CampaignDiscount は自分が持つ判断材料（is_active/start_at/end_at/target_rank）を
使って「自分が今適用可能か」「いくら割引するか」を自分自身で答えられるドメイン
オブジェクトであり、旧CampaignDiscountDataのように呼び出し側へロジックを
漏らさない。これにより3箇所で発生していた重複実装と、その1つが更新から
取り残されて起きた本番バグ（CartPreviewServiceのend_atチェック漏れ）を
構造的に再発しえなくする。
"""
from __future__ import annotations

from dataclasses import dataclass
from datetime import datetime
from decimal import Decimal
from enum import StrEnum


class DiscountType(StrEnum):
    """割引の計算方式。"""

    PERCENT = "PERCENT"
    FIXED = "FIXED"


class CustomerRank(StrEnum):
    """会員ランク。ALL は全ランクを対象とする特別値。"""

    ALL = "ALL"
    GOLD = "GOLD"
    PLATINUM = "PLATINUM"


PERCENT_DIVISOR = Decimal("100")  # パーセント表記(0-100)を小数の割引率に変換する除数


@dataclass(frozen=True, slots=True)
class CampaignDiscount:
    """キャンペーン割引そのものが「対象か」「いくら引くか」を自分で答えられるドメインオブジェクト。

    旧 CampaignDiscountData は公開フィールドだけを持つ「データクラス」で、適用可否判定
    （is_active × end_at × target_rank）と金額計算（PERCENT/FIXED分岐）のロジックが
    呼び出し側の Mgr / CartPreviewService / NotificationService の3箇所に重複していた。
    判断に必要な情報を全て持っているこのオブジェクト自身に振る舞いを持たせることで、
    重複と実装の食い違い（CartPreviewService だけ end_at チェックが漏れていたバグ）を
    構造的に起こりえなくする。
    """

    campaign_id: str
    discount_type: DiscountType
    value: Decimal
    start_at: datetime
    end_at: datetime
    target_rank: CustomerRank
    is_active: bool

    def is_applicable_to(self, customer_rank: CustomerRank, at: datetime) -> bool:
        """このキャンペーンが指定時刻・会員ランクに適用可能かを判定する。

        ガード節で早期returnし、ネストを1段に保つ。

        Args:
            customer_rank: 判定対象の会員ランク。
            at: 判定基準時刻。呼び出し側が注入することで、テストで固定時刻を使える。

        Returns:
            適用可能なら True。
        """
        if not self.is_active:
            return False
        if not (self.start_at <= at <= self.end_at):
            return False
        return self.target_rank == CustomerRank.ALL or self.target_rank == customer_rank

    def discount_amount(self, subtotal: Decimal) -> Decimal:
        """小計に対する割引額を計算する。

        Args:
            subtotal: 割引前の小計金額。

        Returns:
            割引額（円）。

        Raises:
            ValueError: 未知の discount_type が渡された場合（列挙値の網羅漏れを検知する）。
        """
        match self.discount_type:
            case DiscountType.PERCENT:
                return subtotal * (self.value / PERCENT_DIVISOR)
            case DiscountType.FIXED:
                return self.value
            case _:
                raise ValueError(f"未知の割引方式です: {self.discount_type}")


class OrderPricingService:
    """注文確定時の最終金額を計算する。"""

    def calculate_total(
        self,
        subtotal: Decimal,
        customer_rank: CustomerRank,
        discount: CampaignDiscount | None,
        now: datetime,
    ) -> Decimal:
        """割引適用後の注文金額を返す。"""
        if discount is None or not discount.is_applicable_to(customer_rank, now):
            return subtotal
        return subtotal - discount.discount_amount(subtotal)


class CartPreviewService:
    """カート画面のプレビュー金額を計算する。"""

    def preview_total(
        self,
        subtotal: Decimal,
        customer_rank: CustomerRank,
        discount: CampaignDiscount | None,
        now: datetime,
    ) -> Decimal:
        """カート表示用の金額を返す。OrderPricingService と同じ判定ロジックを再利用する。"""
        if discount is None or not discount.is_applicable_to(customer_rank, now):
            return subtotal
        return subtotal - discount.discount_amount(subtotal)


class DiscountNotificationBuilder:
    """Slackへの割引適用通知メッセージを組み立てる。"""

    def build_message(
        self,
        order_id: str,
        subtotal: Decimal,
        customer_rank: CustomerRank,
        discount: CampaignDiscount | None,
        now: datetime,
    ) -> str:
        """割引適用結果を通知文言として返す。"""
        applied_amount = Decimal("0")
        if discount is not None and discount.is_applicable_to(customer_rank, now):
            applied_amount = discount.discount_amount(subtotal)
        return f"割引 {applied_amount}円 を適用しました（注文ID: {order_id}）"

📋 コピーnow = datetime(2026, 8, 31, 10, 0)

expired_campaign = CampaignDiscount(
    campaign_id="SUMMER10",
    discount_type=DiscountType.PERCENT,
    value=Decimal("10"),
    start_at=datetime(2026, 8, 1, 0, 0),
    end_at=datetime(2026, 8, 30, 23, 59),  # 前日に終了済み
    target_rank=CustomerRank.ALL,
    is_active=True,
)

pricing = OrderPricingService()
preview = CartPreviewService()
notifier = DiscountNotificationBuilder()

print(pricing.calculate_total(Decimal("10000"), CustomerRank.GOLD, expired_campaign, now))
# => 10000  （期限切れなので割引なし）

print(preview.preview_total(Decimal("10000"), CustomerRank.GOLD, expired_campaign, now))
# => 10000  （旧実装ではここだけ 9000 になっていた = プレビューと決済金額が食い違うバグ）

print(notifier.build_message("ORD-001", Decimal("10000"), CustomerRank.GOLD, expired_campaign, now))
# => "割引 0円 を適用しました（注文ID: ORD-001）"

# 3箇所すべてが CampaignDiscount.is_applicable_to を呼ぶだけになったため、
# 判定結果は常に一致する。判定ロジックへ将来変更が入っても、修正箇所は
# CampaignDiscount の1メソッドのみで済み、修正漏れによる食い違いは
# 構造的に起こらない。

ポイント解説

実務への応用

MOps 販促システムチームでは、キャンペーン・クーポン・会員ランク特典など「条件付き適用ロジック」を持つマスタデータが多数存在する。今回のように「注文確定処理」「カートプレビュー」「メール文言生成」「Slack通知」など複数の画面・バッチが同じマスタを参照する場面では、判定ロジックをマスタ側のドメインオブジェクトに寄せておくことで、新しい参照箇所（例: 今後追加されるLINE通知）を作る際にロジックのコピペが発生せず、実装漏れによる「表示と実際の挙動が食い違う」系のインシデントを構造的に防げる。

今日のまとめ

次のステップ

• 発展問題: CampaignDiscountを複数同時適用（クーポン＋会員ランク特典の重複適用）に対応させる場合、discount_amountの単純な減算では「割引の重ね掛けで合計が0円未満になる」といった不整合が起きうる。第8章のポリシーパターンで「複数の割引をどの順序・上限でどう合成するか」を切り出す設計を検討せよ
• 参考: 「良いコード・悪いコードで学ぶ設計入門」第1-2章（悪しき構造・設計の初歩）、第3章（カプセル化との関連 — 完全コンストラクタでの不正値排除）

自己評価（あとで記入）