コーディング/アルゴリズム — 販促クーポン管理モジュール リファクタリング

問題

以下は「販促クーポン管理モジュール」の実装です。 このコードには設計上・Pythonイディオムの観点から複数の問題があります。 問題点を列挙し、Python 3.12+ のベストプラクティスで改善してください。

制約・前提条件

• Python 3.12+ を使用すること
• dataclass または @dataclass(slots=True) でクーポンを値オブジェクトとして表現すること
• 「良いコード・悪いコードで学ぶ設計入門」Ch8（条件分岐）・Ch9（コレクション）・Ch10（設計の悪魔）の観点を活用すること
• StrEnum を使ってステータスを型安全に扱うこと
• マジックナンバー（9999999）は名前付き定数化すること

悪いコード (Before)

📋 コピー# --- BAD CODE ---

COUPON_LIST = []

def add_coupon(code, discount, min_order, is_active, expires):
    COUPON_LIST.append({
        "code": code,
        "discount": discount,
        "min_order": min_order,
        "is_active": is_active,
        "expires": expires,
        "used_count": 0,
    })

def apply_coupon(code, order_amount):
    for c in COUPON_LIST:
        if c["code"] == code:
            if c["is_active"] == True:
                if c["discount"] > 0:
                    if order_amount >= c["min_order"]:
                        if c["used_count"] < 9999999:
                            c["used_count"] = c["used_count"] + 1
                            d = order_amount * c["discount"] / 100
                            return order_amount - d
                        else:
                            return None
                    else:
                        return None
                else:
                    return None
            else:
                return None
    return None

def get_active_coupons():
    result = []
    for c in COUPON_LIST:
        if c["is_active"] == True:
            result.append(c)
    return result

def deactivate_expired(now):
    for c in COUPON_LIST:
        if c["expires"] < now:
            c["is_active"] = False

ヒント（段階的開示）

📋 コピーfrom dataclasses import dataclass, field
from enum import StrEnum
from datetime import datetime

MAX_USAGE_COUNT = 9_999_999

class CouponStatus(StrEnum):
    ACTIVE = "active"
    INACTIVE = "inactive"

@dataclass(slots=True)
class Coupon:
    code: str
    discount_rate: int
    min_order_amount: int
    status: CouponStatus
    expires_at: datetime
    used_count: int = field(default=0)

    def is_usable(self, order_amount: int) -> bool:
        """早期return形式"""
        if self.status != CouponStatus.ACTIVE:
            return False
        # ... （各ガード節）
        return True

class CouponRepository:
    def __init__(self) -> None:
        self._coupons: dict[str, Coupon] = {}
    # apply / get_active_coupons / deactivate_expired を実装

問題点分析

リファクタリング後の設計図

模範解答

📋 コピー"""販促クーポン管理モジュール（改善版）。

グローバル状態を排除し、Coupon を値オブジェクト、CouponRepository を
集約ルートとしてカプセル化している。
"""

from __future__ import annotations

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


# --- 定数 ---
MAX_USAGE_COUNT: int = 9_999_999
"""クーポンの最大利用回数（実質的な上限なし）。"""


class CouponStatus(StrEnum):
    """クーポンのアクティブ状態を型安全に表現する列挙型。"""
    ACTIVE = "active"
    INACTIVE = "inactive"


@dataclass(slots=True)
class Coupon:
    """クーポンを表す値オブジェクト。

    Attributes:
        code: クーポンコード（一意）。
        discount_rate: 割引率（0〜100 の整数 %）。
        min_order_amount: 適用可能な最低注文金額（円）。
        status: クーポンの有効状態。
        expires_at: 有効期限。
        used_count: 利用済み回数。
    """
    code: str
    discount_rate: int
    min_order_amount: int
    status: CouponStatus
    expires_at: datetime
    used_count: int = field(default=0)

    def is_usable(self, order_amount: int) -> bool:
        """クーポンが指定注文金額に適用可能かを判定する。

        早期 return で逆条件を先に弾くことでネストを排除している（Ch8）。
        """
        if self.status != CouponStatus.ACTIVE:
            return False
        if self.discount_rate <= 0:
            return False
        if order_amount < self.min_order_amount:
            return False
        if self.used_count >= MAX_USAGE_COUNT:
            return False
        return True

    def calc_discount_amount(self, order_amount: int) -> Decimal:
        """割引額を計算して返す（純粋関数・副作用なし）。"""
        return Decimal(order_amount) * Decimal(self.discount_rate) / Decimal(100)


class CouponRepository:
    """クーポンの永続化・検索・状態変更を担うリポジトリ。

    コレクション自体をカプセル化している（Ch9: コレクションのカプセル化）。
    """

    def __init__(self) -> None:
        # コードをキーにした辞書で O(1) 検索を実現
        self._coupons: dict[str, Coupon] = {}

    def add(self, coupon: Coupon) -> None:
        """クーポンを登録する。"""
        if coupon.code in self._coupons:
            raise ValueError(f"Coupon code '{coupon.code}' already exists.")
        self._coupons[coupon.code] = coupon

    def apply(self, code: str, order_amount: int) -> Decimal | None:
        """クーポンを注文に適用し、割引後金額を返す。"""
        coupon = self._coupons.get(code)
        if coupon is None:
            return None
        if not coupon.is_usable(order_amount):
            return None

        coupon.used_count += 1
        discount = coupon.calc_discount_amount(order_amount)
        return Decimal(order_amount) - discount

    def get_active_coupons(self) -> list[Coupon]:
        """有効なクーポン一覧を返す（内包表記）。"""
        return [c for c in self._coupons.values() if c.status == CouponStatus.ACTIVE]

    def deactivate_expired(self, now: datetime) -> int:
        """有効期限切れのクーポンを無効化し、無効化した件数を返す。"""
        count = 0
        for coupon in self._coupons.values():
            if coupon.expires_at < now and coupon.status == CouponStatus.ACTIVE:
                coupon.status = CouponStatus.INACTIVE
                count += 1
        return count

ポイント解説

Before vs After 比較

実務への応用

• Argo Workflowsのタイムゾーン注入: deactivate_expired(now=datetime.now(tz=JST)) をArgo Workflowsの定期ジョブとして実装する際、now を引数で受け取る設計にしておくとユニットテストで時刻を差し込める。
• Repository パターンの実装切り替え: CouponRepository の _coupons を実際の実装では BigQuery や Cloud Firestore に差し替えることで、テスト時はインメモリ、本番は外部ストレージと切り替えられる。
• 型チェックツールによる安全性向上: apply の返り値を Decimal | None にしたことで、mypy/pyright が「None チェックなしに計算している」コードを検出してくれる。

次のステップ

• 参考: 「良いコード・悪いコードで学ぶ設計入門」改訂新版 Ch8・Ch9・Ch10
• 参考: PEP 663 (StrEnum)、Python decimal 標準ライブラリ

今日のまとめ

自己評価

自分の回答

気づき・メモ