2

次のコード セグメントを検討してください...

 public static UserStatus getEnum(int code) {
    switch (code) {
        case 0:
            return PENDING;
        case 1:
            return ACTIVE;
        case 2:
            return SUSPENDED;
        case 3:
            return DELETED;
        case 4:
            return LOGIN_DISABLED;
        default:
            return null;
        }

}

ケース3とケース4の3番と4番がSONARでマジックナンバーとして検出されるようになりました。

その問題を回避するために、コードセグメントを次のように変更しました...

 public static UserStatus getEnum(int code) {        
    final int Pending=0;
    final int Active=1;
    final int Suspended=2;
    final int Deleted= 3;
    final int Login_details=4;

    switch (code) {
        case Pending:
            return PENDING;
        case Active:
            return ACTIVE;
        case Suspended:
            return SUSPENDED;
        case Deleted:
            return DELETED;
        case Login_details:
            return LOGIN_DISABLED;
        default:
            return null;
    }
}

これは、この種のシナリオでマジック ナンバーの問題を解決する良い方法ですか?.

4

2 に答える 2

7

コードで整数リテラルを使用することは避けたいと考えています。リテラルをメソッドの先頭に移動するだけなので、ソリューションは特に効果的ではありません。定数に意味のある名前を与えるため、少し得になりますが、これらの名前はメソッドにプライベートです。

より良いアプローチは、数字をインターフェイスのフィールドとして定義することです。次に、フィールドを静的にインポートして、定数の記号名として使用できます。

列挙型が定数と同じ順序で宣言されている場合:

enum UserStatus {PENDING, ACTIVE, SUSPENDED, DELETED, LOGIN_DISABLED}

別のトリックを行うことができます：

public static UserStatus getEnum(int code) {
    UserStatus[] values = UserStatus.values();
    return (code >= 0 && code < values.length) ? values[code] : null;
}

ただし、これにより、定数値と列挙型の宣言の間にリンクが作成されます。への呼び出しで実際のパラメータ値が生成される場所によっては、これで問題ない場合がありますgetEnum。

于 2013-03-21T04:27:47.847 に答える
1

問題は単純明快です: 人々があなたのコードを読んでいるとき、なぜ 1 が PENDING を与えるのかは明らかではありません。1 とはどういう意味ですか?

それにセマンティックな意味を与える必要があります。定数を使用することは、通常行うべきことです。

(これが UserService の一部であると仮定するとgetEnum()、コードは次のようになります)

public interface UserService {
   public static final int USER_STATUS_VAL_PENDING = 1;
   public static final int USER_STATUS_VAL_ACTIVE = 2;

   UserStatus getUserStatus(int userStatusVal);
}

public class SimpleUserService implements UserService {
   public UserStatus getUserStatus(int userStatusVal) {
      switch userStatusVal {
      case USER_STATUS_VAL_PENDING:
         return UserStatus.PENDING;
      case USER_STATUS_VAL_ACTIVE:
         return UserStatus.ACTIVE;
      //.....
   }
}

別の回答で示唆されているように、 int-enum マッピングを行うために enum の序数値に頼ることができます。ただし、enum の値を並べ替えたり、末尾から離れた位置に新しい値を追加したりすると、このような方法で問題が発生する可能性があることに注意する必要があります。序数の値が変更され、それをオーバーライドする方法はありません。

注意を払うべきもう 1 つのことは、コードで正しく行われていないことがあるということです。

  1. 目的は整数リテラルを定数にすることでセマンティックな意味を与えることであるため、呼び出し元に見えるものにする必要があるため、ローカル最終変数は正しい方法ではありません。
  2. 定数名には ALL_CAP_UNDERSCORE_DELIMITED を使用する必要があります
于 2013-03-21T06:25:27.690 に答える