Monday, September 01, 2008

A Parameter-Validation Smell and a Solution

Last week, Jeff Fredrick and I did a day-long code review of Platypus. We used a pair-programming approach, with Jeff driving and I helping with the navigation. Eventually, we got into the input parser, which parses input lines into a series of tokens: text, commmands, macros, and comments. Macros can require a second parsing pass, and commands often require additional parsing of parameters.

Once you get a parser working well (that is, it passes unit and functional tests, and it handles errors robustly), you generally don't want to mess with refactoring it. Experience tells you that parsers have hideous code in them and wisdom tells you to leave it alone. However, we launched in.

A frequent cause of otiose code was my extensive parameter checking. Parameters were validated at every step as tokens passed through multiple levels of parsing logic. Likewise, the movement of the parse point was updated multiple tiems as the logic resolved itself back up the processing stack. This too had to be validated repeatedly.

Jeff came up with an elegant refactoring that I could not find in the usual sources. He created an inner class consisting of the passed variables, a few methods for validating them, and a few more methods for manipulating them.

This class was then passed to the methods in lieu of the individual parameters--thereby reducing the number of parameters to one or two. And because the class constructor verified the initialization of the fields, I need only to check whether the passed class was null, rather than validate each of the internal fields.

The effect was to reduce complexity of already complex code, enforce DRY, and place the validation of the variables inside a class that contained them--a set of small, but important improvements. And like many of the best refactorings, it seems obvious in retrospect.

So, if you find your class's methods are repeatedly validating the same parameters, try bundling them in an inner class along with their validation logic. You'll like the results.

5 comments:

Anonymous said...

Sounds like a combination of Introduce Parameter Object and Replace Data Value with Object (Fowler99).

Andrew Binstock said...

@anonymous Yes, you're right. [ blush ] Thank you!

Anonymous said...

I do this when I forced to inherit from an object that has a lot of fields already (like a JComponent). It's much cleaner in the debugger to see all your "own" fields collected in one place.

atul said...

I thought about "Data Clumps" initially - however, Parameter Object is the right thing

- atul

atul said...

I thought about "Data Clumps" initially - however, Parameter Object is the right thing

- atul