Recently I was about to refactor some code Crap4j pointed me to. When I realized most of that code was some kind of switch-case or if-else-cascade, I remembered Daniel´s post and decided to obey those four rules.
This post is supposed to give some inspiration on how to get rid of code like:
switch (value) {
case SOME_CONSTANT:
//do something
break;
case SOME_OTHER_CONSTANT:
//do something else
break;
...
default:
//do something totally different
break;
}
or an equivalent if-else-cascade.
In a first step, let’s assume the constants used above are some kind of enum you created. For example:
public enum Status {
ACTIVE,
INACTIVE,
UNKNOWN;
}
the switch-case would then most probably look like:
switch (getState()) {
case INACTIVE:
//do something
break;
case ACTIVE:
//do something else
break;
case UNKNOWN:
//do something totally different
break;
}
In this case you don’t need the switch-case at all. Instead, you can tell the enum to do all the work:
public enum Status {
INACTIVE {
public void doSomething() {
//do something
}
},
ACTIVE {
public void doSomething() {
//do something else
}
},
UNKNOWN {
public void doSomething() {
//do something totally different
}
};
public abstract void doSomething();
}
The switch-case then shrinks to:
getState().doSomething();
But what if the constants are defined by some third-party code? Let’s adapt the example above to match this scenario:
public static final int INACTIVE = 0; public static final int ACTIVE = 1; public static final int UNKNOWN = 2;
Which would result in a switch-case very similar to the one above and again, you don’t need it. All you need to do is:
Status.values()[getState()].doSomething();
Regarding this case there is a small stumbling block, which you have to pay attention to. Enum.values() returns an Array containing the elements in the order they are defined, so make sure that order accords to the one of the constants. Furthermore ensure that you do not run into an ArrayOutOfBoundsException. Hint: Time to add a test.
There is yet another case that may occur. Let’s pretend you encounter some constants that aren’t as nicely ordered as the ones above:
public static final int INACTIVE = 4; public static final int ACTIVE = 7; public static final int UNKNOWN = 12;
To cover this case, you need to alter the enum to look something like:
public enum Status {
INACTIVE(4),
ACTIVE(7),
UNKNOWN(12);
private int state;
public static Status getStatusFor(int desired) {
for (Status status : values()) {
if (desired == status.state) {
return status;
}
}
//perform error handling here
//e.g. throw an exception or return UNKNOWN
}
}
Even though this introduces an if (uh oh, didn’t obey rule #4), it still looks much more appealing to me than a switch-case or if-else-cascade would. Hint: Time to add another test.
How do you feel about this technique? Got good or bad experiences using it?
Schneide Blog 