Refactoring a long switch statement in Java

Giant switch statements are a code smell. In Java, the usual remedy is to refactor using the Replace Conditional with Polymorphism recipe.

However, I think your problem runs even deeper. I raise the following objections:

• Why should this code exist at all? Shouldn't getCommand(commandType, agentId); just return the appropriate result for all values of commandType? Or are there specific command types that you want to disable? The latter possibility seems disturbing. If this code is, say, part of a permission-checking system to allow only certain types of commands, I would say that the permission-checking system is woefully underdeveloped.
• Why does getCommand(…) return a String? What use could a constant-to-string translation possibly have?

I wish I could give you more constructive suggestions, but unfortunately, you haven't presented much context in the question to allow a deeper analysis.

You could use fallthrough in order to remove the amount of duplicated code.

    switch (commandType) {
        case WHATEV_1:
        case WHATEV_2:
        case WHATEV_3:
        case WHATEV_4:
        case WHATEV_5:
        case WHATEV_6:
        case WHATEV_7:
        case WHATEV_8:
        case WHATEV_9:
            command = getCommand(commandType, agentId);
            break;
        default :
            command = "";
            break;
    }//end switch

Now, different situations are possible :

• you have an enum where each value is properly handled and you have this default just in case something gets added to the enum : this can be detected at compilation time.

• if you have an enum where a handful of known values will lead to command = "" : it might be better to write these cases explicitely and default to command = getCommand(commandType, agentId);.

• if you don't have an enum, maybe you should.

In any case, maybe the switch case could be part of the logic in the getCommand (and return ""). It only moves the issue more that it solves it but I'd expect a switch to be hidden somewhere in that method.

In your code example, all but the default case have the line command = getCommand(commandType, agentId);

You could create a container list or map of all the possible cases, and if commandType does not exist in the container, set the command to an empty string. Otherwise, use the line above to set the command.

@Josay's answer is the best. (I'm just reinforcing it.)

When COMMAND_LOGIN etc. are all compile-time constants, decent compilers will generate a branch table, and execution will be extremely fast. See http://en.wikipedia.org/wiki/Branch_table

I think it is a bad idea to make code more complicated in order to make it shorter. If someone else needs to maintain your code in a few months, they will have an easy time eyeballing your 200-line switch statement, without having to think too hard. The only thing I would improve would be to group commands that use the same getCommand() variant, as Josay describes, so the code looks like this:

switch (commandType) {
    case COMMAND_LOGIN:                 // status-change
    case COMMAND_MAKE_READY:
    case COMMAND_MAKE_NOT_READY:
    case COMMAND_MAKE_NOT_READY_WITH_REASON:
    case COMMAND_LOGOUT_WITH_REASON:
        command = getStatusCommand(commandType, agentId);
        break;

    case COMMAND_ANSWER_CALL:           // call-handling
    case COMMAND_HOLD_CALL:
    case COMMAND_RETRIEVE_CALL:
    case COMMAND_END_CALL:
    case COMMAND_CONSULT_CALL:
        command = getCallCommand(commandType, agentId);
        break;

    case COMMAND_TRASFER_SST_CALL:      // transfer
        command = getTransferCommand(commandType, agentId);
        break;
         . . .
    default:
        command = "";
        break;
}//end switch