0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

@kotaro_ai_labin株式会社Good Lab 開発部

【Javaの良いコード・悪いコード #3】マジックナンバーを定数・enumで消す3原則

0
Posted at

はじめに

株式会社Good Labでエンジニアをしている コータロー です。
日々、Java・SQL・Gitなどの技術情報や、新人エンジニア向けの学習ノウハウ、
AI活用についての情報を発信しています。

Good Labについて気になった方は、コーポレートサイトもぜひご覧ください。
▶コーポレートサイト

この記事は、新人〜2年目のJavaエンジニア向けに 「良いコードと悪いコードの違い」 を、現場でよく見る具体例とともに解説していくシリーズの第3回です。

回 テーマ
#1 命名
#2 コメントの書き方
#3（本記事） マジックナンバー・定数化
#4 Null処理
#5 早期リターン
#6 メソッド分割
#7 ループ処理
#8 例外処理
#9 ログ出力
#10 クラス設計

第3回は マジックナンバー・定数化 です。コードの中に 「ぽつんと書かれた数値」や「ベタ書きの文字列」 は、書いた本人にしか意味が分からない時限爆弾になります。

この記事のゴール

この記事を読み終わると、以下ができるようになります。

  • 「マジックナンバー」「マジックストリング」が何かを説明できる
  • 数値・文字列を static final や enum に切り出す判断ができる
  • 演習問題を通じて、リテラル散在のコードをリファクタリングできる

「マジックナンバー」とは何か

マジックナンバー（Magic Number） とは、コードの中に直接書かれた「意味の伝わらない数値リテラル」のことです。

if (user.getStatus() == 1) {
    sendNotification(user);
}

if (loginAttempts >= 5) {
    lockAccount(user);
}

if (price > 10000) {
    applyDiscount(order);
}

書いた本人は意味が分かっています。
ですが、別のメンバーや3ヶ月後の自分が読んだとき、こう感じます。

  • 「status == 1 の 1 って何？」
  • 「5 ってどこから来た数字？」
  • 「10000 の根拠は？」

このコードを理解するには、他のコード・仕様書・DBの定義 を遡る必要があります。

マジックナンバーの本当のコスト

マジックナンバーには、3つの大きなコストがあります。

  1. 読解コスト：数値の意味を調べるために他の資料を遡る必要がある
  2. 変更コスト：値を変えたいとき、コード中の すべての出現箇所 を直す必要がある
  3. バグコスト：同じ値を別の意味で使っているコードを誤って変更してしまう

特に厄介なのが 変更コスト です。
たとえば「ログイン試行回数の上限を5回から3回に変更したい」というとき、5 という数値が複数箇所にハードコードされていると、全箇所を漏れなく直す 必要があります。

しかも、同じ 5 でも「ログイン試行回数」と「リトライ回数」と「最大ファイル数」など、意味の違う 5 が混在していることもあり、grep だけでは安全に変更できません。

押さえるべきは3原則

新人〜2年目がまず身につけるべき定数化の原則は、以下の3つです。

  1. 意味のある数値は定数（static final）にする
  2. 関連する定数の集合は enum にする
  3. 文字列リテラルも「マジックストリング」として定数化する

順番に見ていきます。

原則① 意味のある数値は定数（static final）にする

意味を持つ数値は、すべて static final で定数化します。

悪い例

public class LoginService {
    public void tryLogin(User user) {
        if (user.getLoginAttempts() >= 5) {
            lockAccount(user);
        }
    }
}

この 5 が何を意味するのか、メソッドを見ただけでは分かりません。
さらに、別のメソッドで同じく 5 を使っていても、それが同じ意味の 5 なのか別の意味の 5 なのかも判断できません。

良い例

public class LoginService {
    private static final int MAX_LOGIN_ATTEMPTS = 5;

    public void tryLogin(User user) {
        if (user.getLoginAttempts() >= MAX_LOGIN_ATTEMPTS) {
            lockAccount(user);
        }
    }
}

MAX_LOGIN_ATTEMPTS という名前から、意味と用途が一目で分かります。
将来 5 を 3 に変えたいときも、定数の宣言箇所を1行変えるだけ で済みます。

定数の書き方ルール

項目 ルール 例
修飾子 private static final（または public static final） private static final int MAX_RETRY = 3;
命名 大文字＋スネークケース MAX_LOGIN_ATTEMPTS、DEFAULT_TIMEOUT_SECONDS
配置 クラスの先頭（フィールド宣言の上）にまとめる フィールドより前
単位 名前に単位を含める（#1で解説した命名ルール） SESSION_TIMEOUT_MINUTES

どんな数値を定数化すべきか

新人が迷うのは「どの数値を定数化すべきか」です。判断軸は以下の通りです。

数値 定数化すべき？ 理由
if (status == 1) の 1 すべき ステータスコードは意味を持つ
for (int i = 0; i < 10; i++) の 0 不要 ループ初期値の 0 は慣用句
price * 0.10 の 0.10 すべき 税率は業務的に意味を持つ
array[0] の 0 不要 先頭要素を指す慣用句
if (count == 0) の 0 不要 「空かどうか」を判定する慣用句
if (age >= 18) の 18 すべき 法的な意味を持つ年齢の閾値

迷ったら、「この数値が変わる可能性はあるか？」 と 「この数値は他のコードでも使われるか？」 を考えましょう。どちらかが「Yes」なら、定数化したほうが安全です。

原則② 関連する定数の集合は enum にする

ステータスやカテゴリのように、関連する複数の定数 がある場合は enum（列挙型）を使います。

悪い例（int定数の羅列）

public class UserStatus {
    public static final int ACTIVE = 1;
    public static final int INACTIVE = 2;
    public static final int SUSPENDED = 3;
}

public class UserService {
    public void notifyUser(User user) {
        if (user.getStatus() == UserStatus.ACTIVE) {
            sendNotification(user);
        }
    }
}

定数化はされていますが、まだ問題があります。

  • 型安全ではない：user.getStatus() の戻り値が int なので、100 や -1 のような不正な値を渡せてしまう
  • IDE補完が効かない：UserStatus. まで打っても候補が出ない（純粋なint定数）
  • 追加・削除に弱い：新しいステータスを足したとき、switch 文の漏れに気づけない

良い例（enumを使う）

User.getStatus() の戻り値も int から UserStatus 型に変更する前提です。

public enum UserStatus {
    ACTIVE, INACTIVE, SUSPENDED
}

public class UserService {
    public void notifyUser(User user) {
        if (user.getStatus() == UserStatus.ACTIVE) {
            sendNotification(user);
        }
    }
}

enum にすることで、以下が手に入ります。

  • 型安全：UserStatus 型以外を渡すとコンパイルエラー
  • IDE補完：UserStatus. まで打つと候補が出る
  • 網羅性チェック：switch 文で全ケースを書き忘れるとIDEが警告

enumに値を持たせるパターン

「会員ランクごとの割引率」のように enumに付随するデータ がある場合、enumにフィールドを持たせます。

public enum MembershipTier {
    REGULAR(0.0),
    GOLD(0.05),
    PLATINUM(0.10);

    private final double discountRatio;

    MembershipTier(double discountRatio) {
        this.discountRatio = discountRatio;
    }

    public double getDiscountRatio() {
        return discountRatio;
    }
}

これで、MembershipTier.GOLD.getDiscountRatio() のように、ランクと割引率がセットで管理 されます。
新しいランクを追加するときも、enum1箇所を直すだけで済みます。

enumを使うべき判断軸

パターン enum を使うべき？
ユーザーステータス（アクティブ、停止、退会） ◎
注文ステータス（受付中、支払い済み、発送済み） ◎
曜日、月 ◎
会員ランク（一般、ゴールド、プラチナ） ◎
エラーコード（数十〜数百種類） △（多すぎる場合は別管理を検討）
業務外のフラグ（true/false） ×

原則③ 文字列リテラルも「マジックストリング」として定数化する

マジックストリング とは、コードの中にベタ書きされた「意味のある文字列」のことです。
数値と同じく、文字列も定数化すべき場面があります。

悪い例

public class AuthService {
    public boolean isAdmin(User user) {
        return user.getRole().equals("admin");
    }

    public boolean canEdit(User user) {
        return user.getRole().equals("admin") || user.getRole().equals("editor");
    }
}

"admin" や "editor" が複数箇所にハードコードされています。問題は3つあります。

  1. タイポに気づけない："amdin" と書いてもコンパイルエラーにならない
  2. 変更コストが高い："admin" を "administrator" に変えるとき、全箇所を直す必要がある
  3. 意味が伝わりにくい："admin" という文字列が「管理者ロール」を意味することがコードから読み取れない

良い例（定数化）

public class Role {
    public static final String ADMIN = "admin";
    public static final String EDITOR = "editor";
    public static final String VIEWER = "viewer";

    private Role() {}
}

public class AuthService {
    public boolean isAdmin(User user) {
        return Role.ADMIN.equals(user.getRole());
    }

    public boolean canEdit(User user) {
        return Role.ADMIN.equals(user.getRole()) || Role.EDITOR.equals(user.getRole());
    }
}

ポイント：

  • Role.ADMIN.equals(user.getRole()) の順で書くと、user.getRole() が null でも NullPointerException にならない（Yoda記法 というイディオム）
  • 定数クラスは private コンストラクタでインスタンス化を防ぐ

さらに良い例（enumに昇格）

ロールの種類が決まっているなら、enum にする方が望ましいです。

public enum Role {
    ADMIN("admin"),
    EDITOR("editor"),
    VIEWER("viewer");

    private final String code;

    Role(String code) {
        this.code = code;
    }

    public String getCode() {
        return code;
    }
}

これなら、DBに保存する値は "admin" のままで、コード上は Role.ADMIN という型安全な値で扱えます。

動作確認：3原則を全部適用したサンプル

3つの原則をすべて適用したコード例です。コピペでそのまま動かせます。

public class MagicNumberDemo {
    private static final int MAX_LOGIN_ATTEMPTS = 5;
    private static final int SESSION_TIMEOUT_MINUTES = 30;
    private static final double CONSUMPTION_TAX_RATE = 0.10;

    public static void main(String[] args) {
        // 原則①：意味のある数値は定数にする
        int loginAttempts = 3;
        if (loginAttempts >= MAX_LOGIN_ATTEMPTS) {
            System.out.println("ログイン試行回数超過");
        } else {
            System.out.println("ログイン試行回数: " + loginAttempts + " / " + MAX_LOGIN_ATTEMPTS);
        }

        // 原則②：enumを使う
        UserStatus status = UserStatus.ACTIVE;
        if (status == UserStatus.ACTIVE) {
            System.out.println("ステータス: アクティブ");
        }

        // 原則③：文字列も定数化
        String userRole = Role.ADMIN;
        if (Role.ADMIN.equals(userRole)) {
            System.out.println("管理者権限あり");
        }

        // 税込計算
        int priceJpy = 1000;
        int taxIncluded = (int) (priceJpy * (1 + CONSUMPTION_TAX_RATE));
        System.out.println("税込価格: " + taxIncluded + "円");
    }
}

enum UserStatus {
    ACTIVE, INACTIVE, SUSPENDED
}

class Role {
    public static final String ADMIN = "admin";
    public static final String USER = "user";
    public static final String GUEST = "guest";

    private Role() {}
}

期待する出力

ログイン試行回数: 3 / 5
ステータス: アクティブ
管理者権限あり
税込価格: 1100円

例外：定数化しなくていいマジックナンバー

「すべての数値を定数化すべき」と教えると、新人は逆に 0 や 1 まで定数化 してしまうことがあります。これはかえって読みにくくなります。

定数化しなくてよい数値の例：

パターン 例 理由
ループの初期値・増分 for (int i = 0; i < list.size(); i++) 慣用句として誰でも読める
配列・リストの先頭 array[0]、list.get(0) 「先頭要素」を意味する慣用句
「空かどうか」判定 if (count == 0)、if (list.size() == 0) 「ゼロ＝空」が自明
単純な計算 width / 2、year + 1 数式として意味が伝わる
テストコード内の値 assertEquals(3, result); 期待値の明示

判断軸：

  • 数値に 業務的・技術的な意味 がある → 定数化する
  • 数値が 数学的・慣用的 に明らかな → そのまま書く

迷ったら、「同じ数値が他の場所にも出てくる可能性があるか？」 を考えましょう。1箇所だけで完結する慣用的な数値は、無理に定数化する必要はありません。

半日溶かした実話：ステータスコード地獄

新ステータスの追加依頼を受けて、ソースをgrepした時のことです。

if (order.getStatus() == 1) {
    // 受付中の処理
}
if (order.getStatus() == 2) {
    // 支払い済みの処理
}
if (order.getStatus() == 3) {
    // 発送済みの処理
}
// ... 同じパターンが何十箇所も

注文ステータスが 1、2、3 という 数値リテラルのまま 何十箇所にも散らばっていました。
新しいステータス「キャンセル待ち」を追加することになり、こんな問題に直面しました。

  • どの数値を割り当てるか（4？5？既存と被らないか）の調査に半日
  • 関連する判定箇所を全部洗い出すために grep を打って 誤検知の確認に1日（数値 1 は他の意味でも使われていた）
  • リリース後、1箇所だけ修正漏れがあって本番でバグ発生

もしステータスが最初から enum で書かれていれば、

  • 新しいステータスを enum に1行足すだけ
  • IDEが switch 文の漏れを警告してくれる
  • 修正漏れは起こりえない

3日かかった作業が、おそらく30分で済んだはずです。

マジックナンバーは、書いた時の楽さと、保守する時の地獄を引き換えにします。

演習問題

難易度の見方

マーク 難易度 目安
⭐ 基本 原則を覚えれば解ける
⭐⭐ 応用 enumへの設計判断が必要

まずは自分で考えてから、模範解答を見てください！

問題1：数値リテラルを定数化する ⭐

次のコードのマジックナンバーを、static final 定数に切り出してください。

public class Sample {
    public static void main(String[] args) {
        int retryCount = 2;
        if (retryCount < 3) {
            System.out.println("リトライ可能: " + retryCount + " / 3");
        }

        System.out.println("キャッシュ有効期限: " + 3600 + "秒");
        System.out.println("パスワード最小長: " + 8 + "文字");
    }
}
模範解答 
public class Exercise01 {
    private static final int RETRY_LIMIT = 3;
    private static final int CACHE_EXPIRES_SECONDS = 3600;
    private static final int PASSWORD_MIN_LENGTH = 8;

    public static void main(String[] args) {
        int retryCount = 2;
        if (retryCount < RETRY_LIMIT) {
            System.out.println("リトライ可能: " + retryCount + " / " + RETRY_LIMIT);
        }

        System.out.println("キャッシュ有効期限: " + CACHE_EXPIRES_SECONDS + "秒");
        System.out.println("パスワード最小長: " + PASSWORD_MIN_LENGTH + "文字");
    }
}

期待する出力

リトライ可能: 2 / 3
キャッシュ有効期限: 3600秒
パスワード最小長: 8文字

ポイント：

  • 定数名は 大文字＋スネークケース（RETRY_LIMIT）
  • 単位を名前に含める（CACHE_EXPIRES_SECONDS、PASSWORD_MIN_LENGTH）
  • クラスの先頭にまとめて宣言する

問題2：複数の関連定数をenumにする ⭐

次のコードは注文ステータスを int で扱っています。
これを enum で書き直してください。switch 文も含めて改善します。

public class Sample {
    public static final int STATUS_PENDING = 1;
    public static final int STATUS_PAID = 2;
    public static final int STATUS_SHIPPED = 3;
    public static final int STATUS_DELIVERED = 4;
    public static final int STATUS_CANCELED = 5;

    public static void main(String[] args) {
        int status = 3;

        if (status == STATUS_PENDING) {
            System.out.println("注文受付中");
        } else if (status == STATUS_PAID) {
            System.out.println("支払い完了");
        } else if (status == STATUS_SHIPPED) {
            System.out.println("発送済み");
        } else if (status == STATUS_DELIVERED) {
            System.out.println("配達完了");
        } else if (status == STATUS_CANCELED) {
            System.out.println("キャンセル");
        }
    }
}
模範解答 
public class Exercise02 {
    public static void main(String[] args) {
        OrderStatus status = OrderStatus.SHIPPED;

        switch (status) {
            case PENDING -> System.out.println("注文受付中");
            case PAID -> System.out.println("支払い完了");
            case SHIPPED -> System.out.println("発送済み");
            case DELIVERED -> System.out.println("配達完了");
            case CANCELED -> System.out.println("キャンセル");
        }
    }
}

enum OrderStatus {
    PENDING, PAID, SHIPPED, DELIVERED, CANCELED
}

期待する出力

発送済み

ポイント：

  • 関連する定数の集合は enum に切り出す
  • if-else if の連鎖は switch 文（Java 14以降は switch 式）に置き換えると見通しが良くなる
  • 新しいステータスを追加したとき、switch の漏れをIDEが警告してくれる

問題3：文字列リテラルを定数化する ⭐

次のコードにはマジックストリングが複数含まれます。
これらを public static final String で定数化してください。

public class Sample {
    public static void main(String[] args) {
        String errorCode = "E001";

        if (errorCode.equals("E001")) {
            System.out.println("リソースが見つかりません");
        } else if (errorCode.equals("E002")) {
            System.out.println("認証エラー");
        } else if (errorCode.equals("E003")) {
            System.out.println("サーバーエラー");
        }
    }
}
模範解答 
public class Exercise03 {
    public static final String ERROR_CODE_NOT_FOUND = "E001";
    public static final String ERROR_CODE_UNAUTHORIZED = "E002";
    public static final String ERROR_CODE_SERVER_ERROR = "E003";

    public static void main(String[] args) {
        String errorCode = ERROR_CODE_NOT_FOUND;

        if (ERROR_CODE_NOT_FOUND.equals(errorCode)) {
            System.out.println("リソースが見つかりません");
        } else if (ERROR_CODE_UNAUTHORIZED.equals(errorCode)) {
            System.out.println("認証エラー");
        } else if (ERROR_CODE_SERVER_ERROR.equals(errorCode)) {
            System.out.println("サーバーエラー");
        }
    }
}

期待する出力

リソースが見つかりません

ポイント：

  • 文字列の比較は 「定数.equals(変数)」 の順で書く（変数がnullでもNullPointerExceptionにならない）
  • 定数名は意味が伝わるように（ERROR_CODE_NOT_FOUND のように内容を表現）

問題4：会員ランクをenumに昇格する ⭐⭐

次のコードは、会員ランクごとの割引率を if-else で分岐しています。
ランクと割引率がセットになっているenum に書き直してください。

前提：会員ランクは「通常」「ゴールド」「プラチナ」の3つ。
割引率は通常0%、ゴールド5%、プラチナ10%。

import java.util.Objects;

public class Sample {
    public static void main(String[] args) {
        System.out.println("通常会員: " + calculateDiscountedPrice(1000, "regular") + "円");
        System.out.println("ゴールド会員: " + calculateDiscountedPrice(1000, "gold") + "円");
        System.out.println("プラチナ会員: " + calculateDiscountedPrice(1000, "platinum") + "円");
    }

    static int calculateDiscountedPrice(int originalPriceJpy, String tier) {
        double discountRatio;
        if (Objects.equals(tier, "regular")) {
            discountRatio = 0.0;
        } else if (Objects.equals(tier, "gold")) {
            discountRatio = 0.05;
        } else if (Objects.equals(tier, "platinum")) {
            discountRatio = 0.10;
        } else {
            throw new IllegalArgumentException("不正な会員ランク: " + tier);
        }
        return (int) (originalPriceJpy * (1 - discountRatio));
    }
}
模範解答 
public class Exercise04 {
    public static void main(String[] args) {
        int regularPrice = calculateDiscountedPrice(1000, MembershipTier.REGULAR);
        System.out.println("通常会員: " + regularPrice + "円");

        int goldPrice = calculateDiscountedPrice(1000, MembershipTier.GOLD);
        System.out.println("ゴールド会員: " + goldPrice + "円");

        int platinumPrice = calculateDiscountedPrice(1000, MembershipTier.PLATINUM);
        System.out.println("プラチナ会員: " + platinumPrice + "円");
    }

    static int calculateDiscountedPrice(int originalPriceJpy, MembershipTier tier) {
        return (int) (originalPriceJpy * (1 - tier.getDiscountRatio()));
    }
}

enum MembershipTier {
    REGULAR(0.0),
    GOLD(0.05),
    PLATINUM(0.10);

    private final double discountRatio;

    MembershipTier(double discountRatio) {
        this.discountRatio = discountRatio;
    }

    public double getDiscountRatio() {
        return discountRatio;
    }
}

期待する出力

通常会員: 1000円
ゴールド会員: 950円
プラチナ会員: 900円

改善ポイント

元のコード 改善後 効果
String tier を引数に取る MembershipTier tier 型で受け取る 不正な文字列を渡せなくなる（型安全）
if-else if で割引率を決定 enumのフィールドとして割引率を保持 ランクと割引率が 常にセット になる
新しいランク追加時に複数箇所を修正 enumに1行追加するだけ 修正漏れがなくなる
throw IllegalArgumentException で不正値検出 型システムで検出されるため不要 実行時エラー → コンパイル時エラー

ポイント：

  • enumに 付随データ（割引率） を持たせるパターンは、業務ロジックでよく使う
  • 「ランク」と「割引率」を別々に管理すると、いつか不整合が起きる
  • enum内にセットで持たせれば、追加・修正時の漏れがゼロになる

まとめ

新人〜2年目が押さえるべきマジックナンバー対策の3原則は、以下の3つです。

  1. 意味のある数値は定数（static final）にする：意味の伝わる名前を付け、変更点を一箇所にまとめる
  2. 関連する定数の集合は enum にする：型安全・IDE補完・網羅性チェックを手に入れる
  3. 文字列リテラルも「マジックストリング」として定数化する：タイポ防止・変更コスト削減

ただし、すべての数値を定数化する必要はありません。
ループの 0 や配列の [0] のような慣用的な数値は、そのまま書いた方が読みやすいです。

判断軸：

  • その数値・文字列が 業務的・技術的な意味 を持つか
  • 複数箇所に出現する または 将来変わる可能性がある か

このどちらかが「Yes」なら、迷わず定数化・enum化しましょう。
書いた瞬間の10秒の手間で、半年後の3日が救えます。

次回予告

次回（#4）は 「Null処理」 を扱います。

  • if (obj != null) の連鎖をどう減らすか
  • Optional の正しい使い方と「やってはいけない使い方」
  • 早期リターンで Null チェックを簡潔にする方法

を、Before / After 形式で解説していきます。

参考

@kotaro_ai_lab
AI活用や開発効率化について発信しています。フォローお気軽にどうぞ！

0
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?