プログラミングの原則：enumの比較はすべてバグ

こんにちは、ソーシャル経済メディア「NewsPicks」のむとうです。

この記事は NewsPicks アドベントカレンダー 2023 の3日目の記事です。

昨日は@J_Nakagawa(隼佑中川)さんによる『LambdaレスポンスストリーミングとAWS-SDKを使ってSlackに進捗バーを表示させる』でした！

世の中には再現が難しく一見してバグがありそうに思えないコードもありますが、一方でプロダクションコードの中にはひと目見てバグが有りそうなコードもまた多いものです。いくつかの特定のパターンをとる文字列(環境名など)やenum(以下どちらもenumと表現します)に関する条件分岐もその一つです。プルリクを見てこのようなパターンがあれば、バグの疑いが強くなります。周囲を見渡すと、大抵すでにバグっているか潜在バグを含むコードが見つかります。すべてバグというのは言い過ぎにせよ、わかりやすさと変更のしやすさの観点から避けるべきであることが多いこともまた事実です。

例えばIaCにおけるこういうコード

// environmentには "production" "dev-1" などの特定の環境名が入る

// 本番だったら、canaryリリース用の設定を作る、バッチを定期実行させる、など
if (envrionment == "production") {
    ...
}

// 開発環境だったら、アクセス制限を追加する、など
if (/^dev-/.test(environment)) {
    ...
}

またはenum値を比較するこういうコードです

// 購読が月間or年間ならば、、？？？
if (subscriptionPlan == SubscriptionPlan.Monthly || subscriptionPlan == SubscriptionPlan.Yearly)

考えてほしいのは、パターンを追加したいときに正しく追加できるかどうかです。ソフトウェアは周囲の状況に応じて変化していくものなので、コードには経年変化に耐え得る頑健性が求められます。このコードはこの部分だけ切り出せば一見問題なく見えるかも知れませんが、実際に変更を加えてみようとするとつらいことが多いです。

本番環境の横に本番相当の検証用環境が必要になったシナリオを考えてみましょう。まずはenvironmentを比較している箇所をIDEで調べてみましょう、、、200箇所見つかりました。検証用環境の名前は、、、そうですね、 previewとでもしましょうか。本番相当とはいえ差異はもちろんあります。さて、では200箇所それぞれについて条件分岐に preview を足すべきかかどうか見ていきましょう、、、

これがなかなかに大変な作業であることは想像に難くないでしょう。すべての条件分岐についてテストが必要ですし、一個一個正しい判断かどうかを検証することも神経をつかいます。何を修正するにしてもいちいち修正箇所を探す手間がかかります。

enum値を比較するケースも見てみましょう。サブスクリプションに「学割プラン」を追加することを考えます。次のコードには学割プランを足すべきでしょうか？そうではないでしょうか？

// 先程のコードの再掲
if (subscriptionPlan == SubscriptionPlan.Monthly || subscriptionPlan == SubscriptionPlan.Yearly)

もちろんこのコードだけでは判断できません。「無料ユーザー以外の有料プランのすべて」を意味しているのか、MonthlyとYearlyだけについて処理をしたいのかはわかりません。後続の処理や周りの文脈をみて判断するしかないでしょう。

こういった条件分岐が行われる場合、名前やenum値は比較対象としての値そのものと比較したいわけではなく、各if文の裏側に何らかの意味が存在していると解釈できます。

環境名の意味と条件式を真偽値表に書き起こすと、このようになります。

名前	canaryが必要か？	バッチが必要か？	アクセス制限をするか？	メールを送信するか？
production	true	true	false	true
preview	false	false	true	false
dev-1	false	true	false	false
dev-2	false	true	false	false

購読プランの意味も同様に考えるとこのように表現できるでしょう。

enum値	課金訴求機能が有効か？	有料限定コンテンツを閲覧可能か？	割引訴求が必要か？	学生証の確認が必要か？
Free	true	false	false	false
Montly	false	true	true	false
Yearly	false	true	false	false
Student	false	true	false	true

複数人で開発している場合、enumを追加する作業をする人と、enumで判断すべき機能を追加する人が違う場合があります。上の表で考えると、enumを追加する人は行を追加し、機能を追加する人は列を追加します。2人の作業がマージされる際には表がもれなく埋まっていなければなりませんが、多くの場合新しい行と列が重なるセルは埋まることがないため、考慮漏れが容易に発生します。

enum値	課金訴求機能が有効か？	有料限定コンテンツを閲覧可能か？	割引訴求が必要か？	学生証の確認が必要か？	新機能
Free	true	false	false	false	false
Montly	false	true	true	false	true
Yearly	false	true	false	false	false
Student	false	true	false	true	true
新プラン	false	true	false	true	考慮漏れ

enumの比較を見つけた際にはこのような考慮漏れを探してみてください。高い確率でバグを見つけることができます。

条件式の意味に名前をつける

行の追加が一箇所の変更で済み、列の追加の際に考慮漏れを発生させなくするには、実際にはenumが意味のある設定項目の集合だということを明確にして、コード上でも名前をつけることです。これらはもともとif文に直接書いてあるような内容なので、マスタデータとしてDBに持つのはオーバーエンジニアリングでしょう。一つ一つを静的なオブジェクトとして定義すると使いやすいです。

言語によって実装手段は様々考えられます。Kotlin(Java)であればenumにプロパティを定義することができるため、表を直接的に表現することができます。

enum class SubscriptionPlan(
    /** 課金訴求機能が有効か？ */
    val isBillingPromotionFeatureEnabled: Boolean,
    /** 限定コンテンツを閲覧可能か？ */
    val isLimitedContentAccessible: Boolean,
    /** 割引訴求を行うか？ */
    val isDiscountPromotionEnabled: Boolean,
    /** 学生証の確認が必要か？ */
    val isStudentVerificationRequired: Boolean,
) {
    FREE(false, false, false, false),
    MONTHLY(true, true, true, false),
    YEARLY(true, true, false, false),
    STUDENT(true, true, false, true),

    /* ↑ではKotlinに慣れていない人向けに表であることをわかりやすく書きましたが
       引数を追加した際に間違いを起こしづらくするため、実際には以下のように名前付き引数にすると良いでしょう。
    FREE(
        isBillingPromotionFeatureEnabled = false,
        isLimitedContentAccessible = false,
        isDiscountPromotionEnabled = false,
        isStudentVerificationRequired = false
    ),
    */
}

そしてif文ではenumの値そのものの比較ではなく属性を使って条件分岐をします。

if (subscriptionPlan.isLimitedContentAccessible) {
    ...

このようなコードは将来的にプランの種類が増えても変更する必要はありません。enumの追加も定義を一箇所追加するだけでおわります。

if文の条件式というところから話を始めたのでここでは値として真偽値だけを持つようなenum型を定義していますが、設定値の集合と解釈するのであればここに色々な値を入れていくことも可能です。アプリケーションの性質に応じてどこまでリッチにするかを判断すべきでしょう。サーバーサイドアプリケーションでは設定値の変更タイミングによっては実際にマスタデータとして外部化するほうがいいかも知れません。IaCの場合ではコードに落ちていることが重要なので、リッチにしていく方向で考えるとより適切でしょう。ニューズピックスのインフラは環境ごとの設定を大きな一つのConfig型として定義し、その中で積極的に型を活用することで柔軟性とメンテナンス性を高度に両立させています。

enumの比較が問題ない場合

最初に述べたようにenumの比較はすべてバグという表現はたしかに大げさではあります。潜在的にマスタデータであるenumを比較することについて注意すべきなのであって、明確に比較対象として意図して定義されたものを比較することに問題はありません。例えばJavaScriptのtypeofの返り値はそれそのものを直接比較することを意図して定義されており、このようなコードはメンテナンス性について問題にはなりません。