そのif-else if、通知手段が増えるたびに関係ない関数まで触ってない?👀

そのif-else if、通知手段が増えるたびに関係ない関数まで触ってない?👀

これ絶対あるじゃん??これ絶対あるじゃん??

  • 通知手段が1個増えるたびに、送信・整形・レート制限の関数を全部書き換えてる
  • if channel == "email" みたいな分岐が、あちこちのファイルに散らばってる
  • 新しいチャネル追加のPRで「このif文、rate_limit.pyにも足して」ってレビューで毎回指摘される
  • 「Discord対応しとこ」でrate_limitのif文を足し忘れ→本番で大量送信してアカウント凍結された

あるじゃん?? マジで絶対あるじゃん??

これ全部、**タイプ(種類)をif文で分岐してるのが原因。**以降「タイプ分岐マシマシ」って呼ぶね。


てかシンプルに言うとねてかシンプルに言うとね

「種類を見てif-else ifで動きを変える」コードのこと。 「メールなら〜、Slackなら〜、SMSなら〜」って1つの関数で全部やってる状態。

「それの何がまずいの?」ってなる気持ち、わかる!
動いてるし、読めば何してるかもわかるし。でもね、新しい種類が増える日から**地獄が始まる。**その話をちゃんとするね。


ちょっと待って、先に全然関係ない話していい?🏠ちょっと待って、先に全然関係ない話していい?🏠

田中さんって経営管理部の広報担当の話、していい?

この会社、社内通達を出す手段がいっぱいあるの。メール、社内掲示板、Teams、社内LINEグループ。その全部を田中さん1人でさばいてた。

「通達の出し方マニュアル」っていうノートが1冊あって、手段ごとにページが分かれてる。文面のテンプレ、送り先の範囲、使っちゃダメなトーン、全部そこに書いてある。


ある日、部長が来て言ったの。

「田中さん、来月から社内Discordサーバー始めるから、Discord用の通達マニュアルも足しといて」

田中さん、ノートを開いて「Discord」のページを書き足した。Discordはカジュアルな空気だから、「みんなー!」みたいなノリの文面テンプレをメモした。書いた勢いで、隣の「メール」ページの文面欄にもそのノリのテンプレがちょっとはみ出しちゃってたの。田中さん、全然気づいてない。


翌朝、経費精算の締切通知を全社員メールで流す仕事が来た。

田中さんはノートの「メール」ページを開いた。

えーっと、メールの文面テンプレは...「みんなー!今月の経費精算、〆切あさってだよ〜 忘れずに!」?

これメールのテンプレだよね? まあそう書いてあるし、そのまま送るか

@company.co.jp の全社員——役員含む——に配信ボタンを押した。

10分後、人事部長から電話。

「田中さん、さっきのメール、役員全員に『みんなー!』で届いてますけど、正気ですか?」

田中さん、メール開いて絶句。

え、わたしそんな文面書いた? ...あ、マニュアルにそう書いてあった...

社長室から内線。「経営会議の前にあれが届いて、かなり微妙な空気になった」

田中さん、その日のうちに全社員と役員に謝罪メールを出すことに。しかもその謝罪メールのテンプレも壊れたままだったから、謝罪メールにも「みんなー!」が混ざってて二重炎上したって。

田中さん、3ヶ月後に異動になった。

これが本番で起きてるやつが、タイプ分岐マシマシってこと。


じゃあコードで見てみよっか👀じゃあコードで見てみよっか👀

さっきの広報担当の話、コードに戻すとこうなるんだよね。田中さんのノート = この3つの関数。

def send_notification(channel: str, user: User, message: str) -> None:
    if channel == "email":
        ses_client.send_email(user.email, subject="お知らせ", body=message)
    elif channel == "slack":
        slack_api.post_message(user.slack_id, message)
    elif channel == "line":
        line_api.push_message(user.line_id, message)
    elif channel == "sms":
        twilio_api.send_sms(user.phone, message)
    elif channel == "discord":
        discord_api.send_dm(user.discord_id, message)
    raise ValueError(f"unknown channel: {channel}")

def format_message(channel: str, template: str, data: dict) -> str:
    if channel == "email":
        return render_html(template, data)
    elif channel == "slack":
        return render_mrkdwn(template, data)
    elif channel == "line":
        return render_plain(template, data)[:500]
    elif channel == "sms":
        return render_plain(template, data)[:70]
    elif channel == "discord":
        return render_discord_md(template, data)
    raise ValueError(f"unknown channel: {channel}")

def get_rate_limit(channel: str) -> int:
    # 1分あたりに送っていい最大件数
    if channel == "email":
        return 100
    elif channel == "slack":
        return 50
    elif channel == "line":
        return 1000
    elif channel == "sms":
        return 10
    # Discordのrate_limit、ここに書くのを忘れた

これ、何が起きるかっていうとね。

  • 「LINE WORKSも追加して」って言われたら、3つの関数ぜんぶにif文を足さなきゃいけない。1個でも足し忘れたら本番で事故る
  • 上のコード、get_rate_limitの中にDiscordの分岐がないじゃん? これで本番にデプロイしたら、Discord通知にレート制限がかからなくて無制限に送りまくる
  • 翌朝、Discordから「お前のBotレート制限超えすぎだからアカウント凍結したわ」って通知が来て、社外パートナーへの連絡がごっそり止まる

わたしも昔これやったから言えるんだけど、「新しいチャネル追加するだけなら30分っしょ」で開いたら、関数3個あちこち触って、気づいたら1日潰れてる。しかも足し忘れで本番落とすやつ。まじで。


やばいのまだあんだけど、バグに気づけない問題😭やばいのまだあんだけど、バグに気づけない問題😭

タイプ分岐マシマシのほんとにヤバいとこは、「壊れてる」じゃなくて「壊れてるのに気づけない」のほうなんだよね。

田中さんのノートで考えてみて? 通達手段が5つある。田中さんがDiscordのテンプレを書き足した。

「他の4つのテンプレが壊れてないか」をチェックするには、4つぜんぶ実際に送ってみて、届いた文面を目で確認しないといけない。

5つなら、まだ頑張れば確認できる。

でも通知手段が10種類あったら? 1個足すたびに10通りの通知を全部送ってみて、届いた画面を目視しないと「他が壊れてないか」わからないじゃん。


コードでも同じことが起きるんだよね。ちょっと見てみて?

さっきの3つの関数をテストするとき、こういうテストケースを書くことになる。

def test_send_notification_email(): ...
def test_send_notification_slack(): ...
def test_send_notification_line(): ...
def test_send_notification_sms(): ...
def test_send_notification_discord(): ...

def test_format_message_email(): ...
def test_format_message_slack(): ...
# ...以下同様

3つの関数 × 5種類のチャネル = 15個のテスト。LINE WORKSを追加したら18個。Chatworkも追加したら21個。

しかも一番ヤバいのは、どこかの関数に分岐を足し忘れても、テストが通っちゃうのよ

send_notificationdiscordの分岐を足した。format_messageにも足した。get_rate_limitに足すのを忘れた。

この状態でtest_send_notification_discordtest_format_message_discordは通る。get_rate_limitのDiscordテストを書き忘れてたら、テストスイートは緑のまま。本番に出て、Discordが凍結された日に初めて発覚するんだよね。

田中さんが「メールのテンプレが壊れてる」のに気づかずに翌朝まで過ごしたのと同じじゃん。分岐を足し忘れた関数は、足し忘れたまま本番に出る。 これがタイプ分岐マシマシの一番怖いとこ。


分けてた会社の話もするね〜✨分けてた会社の話もするね〜✨

田中さんの会社とは別に、最初から通達手段ごとに担当を分けてた隣の会社の話もするね。

この会社、広報チームが通達手段ごとに担当を分けてるの。

  • メール担当:田中さん。メールの文面とテンプレだけ管理してる
  • 掲示板担当:佐藤さん。社内掲示板の投稿だけやる
  • Teams担当:鈴木さん。Teamsへの通達だけやる

それぞれのデスクには、自分の担当手段のマニュアルしか置いてない。田中さんはメールのマニュアルしか持ってないから、Teamsのことを聞かれても「Teams担当はあっち」って指差すだけ。

ある日、社内Discordが始まることになった。この会社はどうしたと思う?

Discord担当を新しく1人配置しただけ。

木村さんって新人がDiscord担当になって、自分用のマニュアルを1冊作った。

田中さんも佐藤さんも鈴木さんも、何も教わってない。 自分のマニュアルには1文字も書き足されてない。

何が起きなかった?

メールのテンプレが壊れなかった。掲示板の文面が壊れることもなかった。全社員メールに「みんなー!」が混ざる事故も起きなかった。役員に謝罪メールを送ることもなかった。田中さんが異動することもなかった。全員がいつも通り仕事して、いつも通り帰ったっていう。


コードに戻すとこうなるんだよね。隣の会社の広報チーム = このクラス群。

from abc import ABC, abstractmethod

# 「通知チャネルってやつは、こういう仕事ができる誰か」っていうルール
class NotificationChannel(ABC):
    @abstractmethod
    def send(self, user: User, message: str) -> None: ...
    @abstractmethod
    def format(self, template: str, data: dict) -> str: ...
    @abstractmethod
    def rate_limit(self) -> int: ...

# メール担当
class EmailChannel(NotificationChannel):
    def send(self, user: User, message: str) -> None:
        ses_client.send_email(user.email, subject="お知らせ", body=message)
    def format(self, template: str, data: dict) -> str:
        return render_html(template, data)
    def rate_limit(self) -> int:
        return 100

# Slack担当
class SlackChannel(NotificationChannel):
    def send(self, user: User, message: str) -> None:
        slack_api.post_message(user.slack_id, message)
    def format(self, template: str, data: dict) -> str:
        return render_mrkdwn(template, data)
    def rate_limit(self) -> int:
        return 50

# SMS担当
class SmsChannel(NotificationChannel):
    def send(self, user: User, message: str) -> None:
        twilio_api.send_sms(user.phone, message)
    def format(self, template: str, data: dict) -> str:
        return render_plain(template, data)[:70]
    def rate_limit(self) -> int:
        return 10

呼ぶ側はこうなる。

def notify(channel: NotificationChannel, user: User, template: str, data: dict) -> None:
    message = channel.format(template, data)
    if exceeds_current_rate(channel.rate_limit()):
        raise RateLimitExceeded()
    channel.send(user, message)

notifyNotificationChannelって書いてあるだけ。中身がメールなのかSlackなのか、気にしてない。渡されたやつに「文面整えて」「何件まで送っていい?」「送って」って聞くだけ。


LINE WORKSを追加したくなったら?

class LineWorksChannel(NotificationChannel):
    def send(self, user: User, message: str) -> None:
        line_works_api.send(user.line_works_id, message)
    def format(self, template: str, data: dict) -> str:
        return render_plain(template, data)
    def rate_limit(self) -> int:
        return 100

クラスを1個作るだけ。 EmailChannelSlackChannelも、1文字も触ってない。

既存のクラスを触らないから「既存の通知が壊れた」っていう事故がそもそも起きない。しかもNotificationChannelの抽象クラスで「この3つのメソッドは必ず実装しろ」ってルールがかかってるから、rate_limitを書き忘れたらLineWorksChannelのインスタンスを作った瞬間にエラーになる。足し忘れが存在しないっつーこと。

テストも、LineWorksChannelの単体テストを書けばOK。既存チャネルのテストは1個も触らない。テストファイル開いて15個の既存テストを眺めて「既存壊してないよね?」って祈る時間がなくなる。


ちゃんとした名前もあるから一応言うね📚ちゃんとした名前もあるから一応言うね📚

ここまで話してきた「種類ごとにクラスを分けて、呼ぶ側は『同じメソッド』で呼ぶ」っていうやつ、ちゃんとした名前がある。

ポリモーフィズム(polymorphism、多態性)。「同じ名前のメソッドを呼んでるのに、呼ばれる相手によって中身の動きが変わる」っていう仕組みのこと。

呼び出す側はchannel.send(user, message)って書くだけ。channelの中身がメールなのかSlackなのかで、実際に動くコードが自動で切り替わる。それがポリモーフィズム。

このリファクタリング自体にも名前があって、「条件分岐をポリモーフィズムで置き換え(Replace Conditional with Polymorphism)」って呼ばれてる。Martin Fowlerって人の『リファクタリング』って本のカタログに載ってるやつ。アンクルボブ(Robert C. Martin)も『Clean Code』で「23個目のにおい」としてこれを紹介してる。

難しそうに聞こえるけど、要は「このif-else if、種類を見てるだけじゃない? 種類ごとに別のクラスに分けたら、if文ごと消えない?」って問い続けるだけなんだよね。


てかこれ、第3回のOCPと同じ話じゃない?ってなった人へ

それ、気づいちゃった? 実は関係ある話なんだよね。

第3回のOCP(開放/閉鎖の原則)は、**「新機能を追加するとき、動いてるコードを書き換えるな」っていう原則の話。**で、今回のポリモーフィズムは、その原則を実現するための具体的な技法の1つ

  • OCP は「触るな」って言ってる(守りたいゴール)
  • ポリモーフィズム は「じゃあ触らないで追加するにはこうするんだよ」って手段(守り方)

OCPが「何を守るか」、ポリモーフィズムが「どうやって守るか」、みたいな関係ってこと。OCPを実現する手段はポリモーフィズム以外にもあるし(プラグイン機構、設定ファイル、DIでもOCPは実現できる)、ポリモーフィズム自体はLSPやDIPみたいな他の原則とも繋がってる。だから「同じ」じゃない。

でも**「if-else ifで種類ごとに分岐してる」っていう症状はガチで同じ。**第3回を読んだ人は「この地獄、見覚えある...」ってなったかも。だいじょぶ、今回は『触るな』じゃなくて『どう触らずに追加するか』の具体的な手順に踏み込む回だから、一歩進んだ話として受け取ってほしい。


これだけ覚えて帰って🥺これだけ覚えて帰って🥺

コードを書くとき・レビューするとき、これだけ問いかけてみて。

「このif-else if、同じ"種類"を見て分岐してるやつが他の関数にもない?」

send_notificationでもformat_messageでもget_rate_limitでも、全部channelを見てif-else ifで分岐してる。同じ種類を何回も見てるなら、それがクラスに分けるサイン。タイプ分岐マシマシの合図ってこと。

もう1個、これも聞いてみて。

「新しい種類を1個増やすのに、既存のコードを何行触る必要ある?」

答えが「3つの関数それぞれに1行ずつ足す」なら、タイプ分岐マシマシ。

答えが「クラスを1個新しく作るだけで、既存のコードは0行」なら、ポリモーフィズムが効いてる状態。


まとめるとこういうことねまとめるとこういうことね

観点タイプ分岐マシマシポリモーフィズム
種類が増えたとき関係する全関数にif-else ifを足す新しいクラスを1個作るだけ
既存コードへの影響既存のif文をぜんぶ読み直す既存クラスは1文字も触らない
足し忘れバグどこかのif文を足し忘れても動いちゃう抽象クラスが強制するから足し忘れが起きない
テスト種類 × 関数の数だけケース爆発新しいクラスの単体テストだけでOK
知識の置き場所1つの関数に全種類の知識が集中クラスごとに自分の種類の知識だけ持つ

「if文を書くな」じゃなくて、**「同じ種類を見てる分岐が2箇所以上に出てきたら、クラスに分ける合図」**っつーこと。


Dev-Here「ギャルでも分かる設計の勘ドコロ!」シリーズ 第16回


沼りたい人はこっちも読んで📖沼りたい人はこっちも読んで📖

書籍

  • Martin Fowler 著『リファクタリング 既存のコードを安全に改善する 第2版』(2019, オーム社) — "Replace Conditional with Polymorphism"を収録したリファクタリングカタログの決定版
  • Erich Gamma ほか著『オブジェクト指向における再利用のためのデザインパターン 改訂版』(1999, ソフトバンクパブリッシング) — Strategy / State パターンなど、ポリモーフィズムを使う古典的なパターン集
  • Robert C. Martin 著『Clean Code』(2008, Prentice Hall) — 「Code Smell G23: Prefer Polymorphism to If/Else or Switch/Case」でこのリファクタリングを紹介

一次資料

  • Martin Fowler の Refactoring カタログ: https://refactoring.com/catalog/ — "Replace Conditional with Polymorphism" を含む全リファクタリングの一覧

無料で読める入門記事