A コーディング — コレクションのカプセル化・ループネスト解消（バナー表示対象商品フィルタ Bad→Good Ch9）

概要

問題

EC企業 MOps 販促システムチームでは、トップページ・カテゴリページに表示するバナー表示対象商品を、対象商品マスタ・実施中キャンペーン・除外カテゴリの3つの条件から抽出するfilter_banner_targetsを運用している。抽出結果はcampaign_id単位でインメモリキャッシュ（_target_cache）され、次回以降の表示リクエストで再利用される。

さらに調査したところ、除外カテゴリoutletに属する商品が、条件上は除外されるはずなのにバナーに紛れ込むケースが以前から散発しており、原因不明のまま放置されていた。

「良いコード・悪いコードで学ぶ設計入門」第9章（コレクション：ループネスト解消、コレクションのカプセル化）を使って、この2つの不具合の根本原因を特定し、リファクタリングしてください。

制約・前提条件

• Python 3.12+ を前提とする（dataclass(slots=True)、frozenset、モダンな型ヒントを使ってよい）
• product・campaignは生のdictではなく、@dataclass(frozen=True, slots=True)の値オブジェクトとして表現すること
• PromotionTargetProducts（対象商品コレクション）は、外部から内部のリストを直接取得・変更できないファーストクラスコレクションとして再設計すること（読み取りは可能だが、呼び出し側の.append()などで内部状態が汚染されないこと）
• 3重・4重のネストループを解消し、各商品につき1回のループで判定できる構造にすること（除外カテゴリ判定は集合演算を使い、inによるO(n)の線形探索を避けること）
• 「除外カテゴリの商品が紛れ込む」バグの原因を1文で説明すること
• 未使用のtag_priority引数の扱いについても言及すること
• Google スタイル docstring・インラインコメント・名前付き定数を含めること

悪いコード (Before)

📋 コピーclass PromotionTargetProducts:  # 問題4: 実質ただのlistラッパー
    """バナー表示対象商品を保持するクラス"""

    def __init__(self):
        self.products = []

    def add(self, product):
        self.products.append(product)

    def get_products(self):
        return self.products  # 問題2: 内部の生リストをそのまま返す


def filter_banner_targets(products, campaigns, excluded_categories, tag_priority):
    """バナー表示対象商品を、在庫・価格・カテゴリ条件で抽出する"""
    targets = PromotionTargetProducts()
    for product in products:
        for excluded in excluded_categories:  # 問題1: このcontinueは
            if product["category"] == excluded:                #        excludedループにしか効かない
                continue  # 商品全体をスキップしたつもりが素通りする
        for campaign in campaigns:  # 問題3: product×excluded×campaignsの多重ループ
            if product["category"] in campaign["target_categories"]:
                if product["stock"] > 0:
                    if product["price"] >= campaign["min_price"]:
                        if product not in targets.get_products():  # 問題3: 呼び出す
                            targets.add(product)                #        たびO(n)線形探索
    return targets.get_products()


_target_cache = {}


def get_banner_targets(campaign_id, products, campaigns, excluded_categories, tag_priority):
    # 問題5: tag_priorityは受け取るだけで一度も使われない
    if campaign_id not in _target_cache:
        matched_campaigns = [c for c in campaigns if c["id"] == campaign_id]
        result = filter_banner_targets(products, matched_campaigns, excluded_categories, tag_priority)
        cache_entry = PromotionTargetProducts()
        for p in result:
            cache_entry.add(p)
        _target_cache[campaign_id] = cache_entry
    return _target_cache[campaign_id].get_products()  # 問題2続き: 呼び出し側が
    # .append()すると_target_cache本体まで書き換わる(エイリアシング)

# 問題6: product["category"]のような生dictアクセスはタイポ・キー欠落で
#         実行時までKeyErrorに気づけない

ヒント（段階的開示）

@dataclass(frozen=True, slots=True)
class Product:
    id: str
    category: str
    stock: int
    price: int

class BannerTargetProducts:
    def __init__(self, products: tuple[Product, ...] = ()) -> None:
        self._by_id: dict[str, Product] = {p.id: p for p in products}  # id基準で重複排除

    def added(self, product: Product) -> "BannerTargetProducts":
        return BannerTargetProducts((*self._by_id.values(), product))  # 不変: 新インスタンスを返す

    def __iter__(self):
        return iter(self._by_id.values())

def filter_banner_targets(products, campaign, excluded_categories: frozenset[str]) -> BannerTargetProducts:
    targets = BannerTargetProducts()
    for product in products:
        if product.category in excluded_categories:
            continue  # 単一ループ内なので、意図通りこの商品をスキップできる
        # ... 残りのガード節

問題点分析（6点）

構造の比較（SVG）— 多重ループ+内部漏洩 vs ガード節+ファーストクラスコレクション

模範解答

📋 コピー"""banner_target_products.py — コレクションのカプセル化・ループネスト解消（Ch9）

トップページ・カテゴリページのバナー表示対象商品を抽出する処理を、
「continueが外側ループに効かず除外カテゴリが漏れる」「内部リストの参照を
そのまま返しキャッシュが外部から汚染される」という2つの不具合から、
ファーストクラスコレクション+ガード節によるループ平坦化へ置き換える。
"""
from __future__ import annotations

from dataclasses import dataclass
from typing import Final, Iterator

_MIN_STOCK: Final[int] = 1  # 在庫「あり」とみなす最小数量


@dataclass(frozen=True, slots=True)
class Product:
    """商品マスタの1件を表す値オブジェクト。"""

    id: str
    category: str
    stock: int
    price: int


@dataclass(frozen=True, slots=True)
class Campaign:
    """実施中キャンペーンの1件を表す値オブジェクト。"""

    id: str
    target_categories: frozenset[str]
    min_price: int


class BannerTargetProducts:
    """バナー表示対象商品のファーストクラスコレクション。

    生の`list`を外部に一切晒さず、「追加」「重複排除」「反復」のみを許可する。
    追加は`added()`で新しいインスタンスを返す不変設計とし、キャッシュとして
    保持している既存インスタンスが呼び出し側から書き換わる余地をなくす(修正2)。
    """

    def __init__(self, products: tuple[Product, ...] = ()) -> None:
        # idをキーにした辞書で保持し、追加のたびにO(1)で重複排除する(修正3)
        self._by_id: dict[str, Product] = {p.id: p for p in products}

    def added(self, product: Product) -> BannerTargetProducts:
        """商品を1件追加した新しいコレクションを返す。

        Args:
            product: 追加対象の商品。

        Returns:
            追加後の新しい BannerTargetProducts。自身は変更しない(不変)。
        """
        return BannerTargetProducts((*self._by_id.values(), product))

    def __iter__(self) -> Iterator[Product]:
        return iter(self._by_id.values())

    def __len__(self) -> int:
        return len(self._by_id)

    def as_tuple(self) -> tuple[Product, ...]:
        """読み取り専用のタプルとして公開する。ミュータブルな`list`は返さない(修正2)。"""
        return tuple(self._by_id.values())


def filter_banner_targets(
    products: tuple[Product, ...],
    campaign: Campaign,
    excluded_categories: frozenset[str],
) -> BannerTargetProducts:
    """バナー表示対象商品を、在庫・価格・カテゴリ条件で抽出する。

    Args:
        products: 判定対象の全商品。
        campaign: 対象キャンペーン(1件)。複数キャンペーンは呼び出し側でループする。
        excluded_categories: 除外カテゴリの集合(O(1)判定のためfrozenset)。

    Returns:
        条件を満たす商品のみを保持する不変コレクション。

    Note:
        `tag_priority`はBad版で未使用のまま渡されていた引数(修正5)。
        バナー表示順序に使う想定であれば、抽出結果(BannerTargetProducts)を
        別関数でソートする責務に切り出すべきで、抽出ロジックに混ぜ込まない。
        現時点で利用予定が無いなら引数ごと削除するのが正しい対応。
    """
    targets = BannerTargetProducts()
    for product in products:
        # 修正1: 単一ループ内のガード節なので、この商品自体を正しくスキップできる
        # (Bad版はexcluded_categoriesの内側ループでcontinueしており、外側のproduct
        #  ループには効いていなかった)
        if product.category in excluded_categories:
            continue
        if product.category not in campaign.target_categories:
            continue
        if product.stock < _MIN_STOCK:
            continue
        if product.price < campaign.min_price:
            continue
        # 修正3: BannerTargetProducts内部でid重複排除するため、ここでのO(n)線形探索は不要
        targets = targets.added(product)
    return targets


_target_cache: dict[str, BannerTargetProducts] = {}


def get_banner_targets(
    campaign_id: str,
    products: tuple[Product, ...],
    campaigns: tuple[Campaign, ...],
    excluded_categories: frozenset[str],
) -> BannerTargetProducts:
    """キャンペーンIDごとにバナー表示対象商品をキャッシュしつつ返す。

    Args:
        campaign_id: 対象キャンペーンID。
        products: 商品マスタ全件。
        campaigns: 実施中キャンペーン全件。
        excluded_categories: 除外カテゴリの集合。

    Returns:
        読み取り専用のBannerTargetProducts。呼び出し側が要素を追加しても
        キャッシュ本体(_target_cache)は汚染されない(修正2)。
    """
    if campaign_id not in _target_cache:
        campaign = next(c for c in campaigns if c.id == campaign_id)
        _target_cache[campaign_id] = filter_banner_targets(products, campaign, excluded_categories)
    return _target_cache[campaign_id]

📋 コピーproducts = (
    Product(id="P1", category="shoes", stock=5, price=6000),
    Product(id="P2", category="bags", stock=0, price=8000),   # 在庫0で除外
    Product(id="P3", category="outlet", stock=3, price=1000), # 除外カテゴリ
    Product(id="P4", category="shoes", stock=2, price=12000),
)
campaign = Campaign(id="SUMMER2026", target_categories=frozenset({"shoes", "bags"}), min_price=5000)
excluded = frozenset({"outlet"})

targets = get_banner_targets("SUMMER2026", products, (campaign,), excluded)
[p.id for p in targets]
# => ['P1', 'P4']
# P2は在庫0で除外、P3は除外カテゴリで正しく除外(Bad版のバグは再発しない)

# フロントエンド担当が「もっと見る」ダミーを足したいケース
more_link = Product(id="MORE_LINK", category="__more__", stock=1, price=0)
display_targets = targets.added(more_link)

len(targets), len(display_targets)
# => (2, 3)
# targets(キャッシュ本体)は変更されない。display_targetsは新しい別インスタンスなので、
# 次回同じcampaign_idで get_banner_targets() を呼んでもMORE_LINKは混入しない

ポイント解説

実務への応用

• MOps販促システムチームでは、バナー表示対象商品の他にも「レコメンド対象商品」「クーポン適用対象商品」「在庫引当対象商品」など、複数条件でフィルタしたコレクションをキャッシュして使い回す処理が頻出する: 今回のBannerTargetProductsのような「内部の生コレクションを晒さないファーストクラスコレクション」は、そのまま横展開できる汎用パターン
• 「キャッシュに入れたオブジェクトを、取得側が誤って書き換えてしまう」バグは、Redisやローカルキャッシュ（functools.lru_cache等）を使う処理全般で起こりうる: ミュータブルなオブジェクトをキャッシュする場合は、常に「取得側から見えるのはコピーか、不変オブジェクトか」を確認する習慣が有効

今日のまとめ

次のステップ

• 発展問題: BannerTargetProductsに、表示順序（tag_priorityベースのソート）を加えたい場合、抽出ロジック（filter_banner_targets）と表示順序の決定ロジックをどう責務分割するか設計せよ（第6章「関心の分離」との関連）
• 発展問題: _target_cacheをプロセス内辞書ではなくRedisに置き換える場合、BannerTargetProductsのシリアライズ/デシリアライズ時に不変性・重複排除の保証をどう維持するか検討せよ
• 参考: 「良いコード・悪いコードで学ぶ設計入門」第9章（コレクション）、第4章（不変の活用）との関連、Python公式ドキュメント dataclasses、frozenset

自己評価（あとで記入）