Saturday, March 29, 2008

One Bad Attribute

I spend a lot of time thinking about unit tests. Probably because I spend a lot of time writing them. Oh, that warm feeling of seeing a class with 100% coverage... unbeatable. I've also recently been spending a lot of time at system boundaries; those places like file systems and Corba (org.OMG.Corba never stops amusing me), places where the type system gets a little muddy and you find yourself passing around maps and lists of strings instead of typed values.

These are places where testing error conditions can be a hassle. If you have a method that takes a map of key value pairs and expects there to be 15 or so entries, are you really going to test the contract on that method? Test that an IllegalArgumentException is thrown for each of the 15 scenarios where one of the entries in the map is missing? Oh, you don't? Can't blame you. You do? It's good to see others with the same compulsive disorder as me.

So constructing a map with one invalid value in 15 different ways is painful. It leads to a lot of repetition (not to mention duplication!). You either repeat the 14 good values each time in each test method, or maybe you create a factory method that has a bunch of complex conditionals to leave out a default value. What a headache. What you need is the One Bad Attribute pattern. Simply put: create the complex data object in a valid state, and then take away a required value to put it in an invalid state. This sure beats trying to create an invalid object in one shot.

Check it out, here's a class constructor we're trying to test (example in Groovy, natch):

class SystemUnderTest {
SystemUnderTest(map) {
if (!map.containsKey("GUID")) throw new IllegalArgumentException()
if (!map.containsKey("VERSION")) throw new IllegalArgumentException()

//rest of method ommitted
}
}

And here is the GroovyTestCase testing the constructor contract:

class BadAttributeTest extends GroovyTestCase {

def validMap = [ GUID: "...",
VERSION: "..."]

void test_constructor_contract_guid() {
shouldFail(IllegalArgumentException) {
validMap.remove("GUID") //one bad attribute
new SystemUnderTest(validMap)
}
}

void test_constructor_contract_version() {
shouldFail(IllegalArgumentException) {
validMap.remove("VERSION") //one bad attribute
new SystemUnderTest(validMap)
}
}
}

So what are the benefits of doing this? I find the one bad attribute pattern intent revealing because there is one clear definition of the valid parameter, and in each test method there is a clear definition of what makes this version of that object invalid. Show what matters, hide what doesn't, and Don't Repeat Yourself. In this particular example, my map only contains two items... so maybe this isn't that impressive right now. But when maps and such contain many more items I find this pattern really shines.

I'll leave you with a bit of One Bad Attribute trivia: The pattern name is usually pronounced with a Mexican accent: "That is one bad attribute, amigo.", and the pattern name is never abbreviated to OBA. While this trivia is true (I guarantee it!), I don't have a definitive source. If anyone can explain the history of this then please leave a comment.

No comments: