Solution for Magic Number issue......?

Ruchira Gayan Ranaweera picture Ruchira Gayan Ranaweera · Mar 21, 2013 · Viewed 13.2k times · Source

Consider the following code segment...

 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;
        }

}

Now number 3 and 4 in cases(case 3 and case 4) are detected as magic numbers by SONAR.

To avoid that issue I changed my code segment as follows...

 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;
    }
}

Is this a good way to solve the magic number issue in this kind of scenario ?.

Answer

Ted Hopp picture Ted Hopp · Mar 21, 2013

I gather that you want to avoid using integer literals in the code. Your solution is not particularly effective because it simply moves the literals to the top of the method. It gains a little bit because it gives meaningful names to the constants, but these names are private to the method.

A better approach would be to define the numbers as fields in an interface. You can then statically import the fields and use them as symbolic names for the constants.

If the enum is declared in the same order as the constants:

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

you can do another trick:

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

However, this creates a linkage between the constant values and the declaration of the enum. This may be okay, depending on where the actual parameter values are generated in calls to getEnum.