HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Long switch statement to lookup fifty-something commands

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
statementcommandslonglookupswitchfiftysomething

Problem

I have a very long switch statement which is expected to have nearly 50-55 cases.

switch (commandType) {
            case COMMAND_LOGIN:
                command = getCommand(commandType, agentId);
                break;

            case COMMAND_MAKE_READY:
                command = getCommand(commandType, agentId);
                break;

            case COMMAND_MAKE_NOT_READY:
                command = getCommand(commandType, agentId);
                break;

            case COMMAND_MAKE_NOT_READY_WITH_REASON:
                command = getCommand(commandType, agentId);
                break;

            case COMMAND_LOGOUT_WITH_REASON:
                command = getCommand(commandType, agentId);
                break;

            case COMMAND_ANSWER_CALL:
                command = getCommand(commandType, agentId);
                break;

            case COMMAND_HOLD_CALL:
                command = getCommand(commandType, agentId);
                break;

            case COMMAND_RETRIEVE_CALL:
                command = getCommand(commandType, agentId);
                break;

            case COMMAND_END_CALL:
                command = getCommand(commandType, agentId);
                break;

            case COMMAND_TRASFER_SST_CALL:
                command = getCommand(commandType, agentId);
                break;

            case COMMAND_CONSULT_CALL:
                command = getCommand(commandType, agentId);
                break;
                 .
                 .
                 .
                 .
                 .
           default :
                command = "";
                break;

        }//end switch


It doesn't seem to be something which should exist in this current form, I have gone through some questions here 'but they doesn't seem to help in this condition' I have seen the "strategy pattern", but that also seems to be suitable for the case where i have to execute different method for every case. Kindly guide me how the s

Solution

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.

Context

StackExchange Code Review Q#74743, answer score: 10

Revisions (0)

No revisions yet.