I’m refactoring code and have reached a horribly gigantic switch statement. Every single API method available to end users is represented as an enum and we have a switch statement iterating over the enum and responding to each possible API call.
Each individual enum option has only a few lines of code. Any sufficiently complicated command gets a method call that is made. However, with so many different enums ever a half dozen lines per enum grows to be rather huge.
What is the correct approach to make this cleaner? I could write a unique method for every command, but I already moved all the complicated functionality to their own classes. Does having a new method for a 4-5 lines of code only ever called once really add that much additional cleaneness to the code as a whole?
It seems rather tedious to create unique methods for all these cases, and the switch would still be a bit ugly even if it only allocated two lines to each enum (method call and break). Is there a better way to avoid the switch statement entirely?
How would one try to keep this clean?
ps. this is in java, functional programing is clearly not an option.
6
My advice would be to leave well alone.
A switch is the most efficient possible branching mechanism especially when coupled with an enum.
Anything else will incur a performance hit.
It may well be worth factoring out the inline code into separate methods for readability ( The JIT compiler will probably just inline them again — so no overhead here ).
I personally find nothing wrong with two or three line methods if it helps readability.
Also for readability you could consider putting every case on a single line like so:—-
switch {humungous_ENUM) {
case OPTION_1: exOption_1(); break;
case OPTION_2: exOption_2(); break;
......
case OPTION_99: exOption_99(); break;
default: exOption_error();
}
2
This is coming from a C# perspective, but what I would do in that case is the following:
- Refactor all of the “small bits of code” into separate methods.
- Create a dictionary which is keyed on the enum and which contains an Action delegate (a delegate which returns void and is parameterless).
- Create a method which adds each enum/delgate pair to the dictionary.
- Use the dictionary as a lookup to execute the correct code.
And for more advanced users:
- Create an attribute which takes the enum value as a parameter. On load, reflect over the assembly and automatically add each enum value/method pair to the dictionary (this will save on all the “adding to dictionary” code).
8
You say each case in your big switch corresponds to a public (also published?) API method. From this, I deduce that calls to those API methods are, in reality, calls to some common function with the corresponding enum parameter. I also assume you have the capability to change this public API interface.
It doesn’t matter the size of those methods; if they are independent, they should be isolated in separate, public functions. This is true for methods implementing unrelated behaviour, and is orthogonal to their implementation in itself (those classes you mention having all the complicated stuff). A different thing is having several methods with similar behaviour, in which case a single, parameterized method maybe could be added instead. The main guideline in order to reduce the quantity of public functions would be avoiding behaviour duplicity on them.
Last, but not least, try to group semantically related methods (perhaps using static classes), in order to bring in some structure to the otherwise flat array of functions.