Saturday, November 04, 2006

Enums are a good thing

Yesterday I dove into the tests looking for something to clean up. I started with the NonUniqueObjectException we're getting in one test (and swallowing), but in the process of trying to look around to see what the two objects might be that make uniqueness not exist, I found other code smells.

So I'm looking at code which (simplified) looked something like:

createClient(Short.valueOf("3"), "A test client")

The pain involved in using short instead of int is the first glaring thing, but actually what that really should have been was an enum:

createClient(CustomerStatus.CLIENT_ACTIVE, "A test client")

If those two things look basically the same to you, I'd suggest thinking a little harder about where you are spending your brain power while reading/maintaining this code. Sure, once you've come up to speed you can probably remember that "3" here means active, but shouldn't you have the computer keep track of that? And if you are just learning this code, or forgot that detail, then "3" is totally mystifying - in fact what got me onto this tangent is that I was wondering whether it was an ID which, duplicated, had something to do with the Hibernate non-unique exception.

One more detail: how did I fix this? The createClient method had about 150 callers (fortunately with good test coverage). So I didn't want to fix them all at once. I created my new createClient:

createClient(CustomerStatus status, String name)

and had it call the old one (or maybe vice-versa, the point is having one call the other rather than a copy-paste, since it is so easy to look up the enum from the short, or vice-versa):

createClient(short status, String name)

I then started fixing up callers. I think I got to about 100 before I got bored. So I checked in the 100, and I can get to the other 50 some other day.

I suppose I could also turn this into a rant about how helpful Java's strong typing is, because with the enum I know (as I'm typing, thanks to Eclipse, not just at run-time) what that first argument to createClient is. But that's a debate which goes back at least to the 1960's. I'll just say that since we are paying the price (extra syntax, mainly) for compile-time types, we should get the payoff.

No comments: