Monday, July 09, 2007

Switch Considered Harmful - Advanced enums use instead

Whenever I see a switch statement in object-oriented code, an alarm bell goes of. Most of the time, it will be a code-smell, which should be expressed through the use of polymorphism instead. With Java5 enums, you can get around switch-logic, by adding per enum constant methods.

Consider the code below:

enum LogLevel { ERROR, WARN, INFO }

private void doLog(LogLevel logLevel, String msg) {
String prefix;
switch (logLevel) {
case INFO:
prefix = "INFO";
break;
case WARN:
prefix = "WARN";
break;
case ERROR:
prefix = "ERROR";
break;
default:
throw new RuntimeException("Oops, forgot to add stuff on new enum constant");
}
System.out.println(String.format("%s: %s", prefix, msg));
}

...

// and its usage..
doLog(LogLevel.WARN, "Dude, where's my car?");
The example is dumb, I know, but it illustrates how the switch is ugly. If we add a new enum constant like DEBUG to LogLevel, we would have to remember to update all places where the enum is switched upon.

Here is a better solution:

enum LogLevel {
ERROR {
void log(String msg) {
doLog("ERROR", msg);
}
},
WARN {
void log(String msg) {
doLog("WARN", msg);
}
},
INFO {
void log(String msg) {
doLog("INFO", msg);
}
};

abstract void log(String msg);

private static void doLog(String prefix, String msg) {
System.out.println(String.format("%s: %s", prefix, msg));
}
}

...

// and its usage..
LogLevel.WARN.log("Dude, where's my car?");
Here, the "log logic" is put onto each enum constant itself and we dispatch to that logic by calling the log method on the enum variable directly. If we were to add DEBUG to LogLevel, we would be forced to implement the abstract log method for that constant too.

6 comments:

Ricky Clarkson said...

The problem with this approach is that not everything you might want to do with an enum constant is appropriate to place into its source file.

To make a ridiculous example, consider enum Bool { TRUE, FALSE }.

It wouldn't make sense to place everything that might depend on a Bool into Bool.java. Less ridiculous examples just take more thinking about, but the same conclusion can be reached.

In languages where you can add methods outside the class' source file, such as Ruby and Lisp, the enum (or some equivalent) is appropriate.

Also consider that what you're aiming for is Haskell-like pattern matching, not really enums.

Per Olesen said...

I see your point! The code I show is only applicable for enums that are strictly bound to the problem domain. Not to general ones like Boolean.

On the issue of "haskell-like pattern matching", I will just have to take your word on that one :-)

Ricky Clarkson said...

The problem with switch on enums is twofold:

1. It isn't guaranteed to cover all cases. IDEs can help with this, though you do need to have it recompile/reanalyse dependencies to have the IDE notice any additional cases.

2. It is a statement, not an expression. That is, you can't do:

System.out.println(switch (value)
{
. . case LOW: "A low value";
. . case MED: "Somewhere in the middle";
. . case HIGH: "A high value";
}

(which is almost pattern matching - most/all pattern matching implementations are smarter than this)

Instead, you end up storing what should be the value of an expression in a temporary variable, repeating more than you should need to, and being less expressive.

Dantelope said...

Point aside, the example was a bit frivolous. After all, the obvious solution to this specific problem -- at least as presented -- is:

enum LogLevel { ERROR, WARN, INFO }

private void doLog(LogLevel logLevel, String msg)
{ System.out.println(String.format("%s: %s", logLevel.name(), msg);
}

:-)

Shane said...

I don't know what switch defenders are complaining about your post. Java enumeration pattern has been long adopted with the book "Effective Java Programming". I think this is the only sane way of controlling your code. Sure, you can always find some ridiculous example like Boolean. But seriously, "HUH?"

With enum in Java you don't need to do the house keeping of toString or equals or hash code anymore, which I think is really cool.

Anonymous said...

Shane
You don't get it, this was a bad example, a misuse of enums and fragile.
Actions should be done via maps keyed by enums not direct from an enum method, that way you can use the same set of enums for different logger implementions.