CodeSOD: Enumerating Your Failures

Rick was recently looking at some code from another team at his company. He noticed something odd in the code, so he pinged the team lead, Linda. "Did you spot this?"

"Oh, crap no," Linda replied. "I should have caught this in code review, but I gotta be honest, Teddy is a bit" well, let's just say I really should have caught that since I knew it was a Teddy commit."

Rick didn't know much about Teddy, beside the fact that Teddy had a few years of experience. Certainly, he had enough experience to be familiar with Java programming, like working with enums and working with unit tests. Unfortunately, he doesn't seem to understand them.

public class ProductCodeEnumTest{ @Test public void testProductCodeEnum_ProductCodeEnumExists() { String[] actualResults = {"PRODUCT_CODE_A","PRODUCT_CODE_B"}; ProductCodeEnum[] expectedResults = ProductCodeEnum.values(); for (int i=0; i<actualResults.length; i++) { assertEquals("Expected results do not equal actual results.", expectedResults[i].getValue(), actualResults[i]); } }}

This unit test confirms that the ProductCodeEnum starts with PRODUCT_CODE_A and PRODUCT_CODE_B. Is that a useful test? Of course not! Did Teddy write a test like this for every enum in the code base? Well, he tried to, but Linda usually caught them in code reviews.

Maybe Teddy wanted to make sure other developers didn't change the enum, but this isn't the way to do it, and also, why would you worry about preventing changes to the enumerated type? If another developer removed a used entry, you'd get a compile time error. If a developer adds a new entry, it probably belongs there. If another developer changes the order of the entries, that shouldn't matter at all. None of this matters.

I suppose this makes their code coverage metrics look better, at least.

Rick supplies his own enumeration of Teddy's failures, and predicts the comments on this article:

Looking forward to reader comments regarding:
* the pointlessness of unit tests of enums (or the old sweats pointing out the couple of valid edge cases)
* the reversed names of actual vs expected results
* the useless message in assertEquals
* the fact that the test requires the enum to be in a certain order to pass
* the fact that the test will still pass if extra values are added to the enum
* the oddly double-up class name (yes it really is named in the format testEnumName_EnumNameExists)
Are there any other code smells I missed?

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!