Homepage ->  Individual Weblogs -> Kevin Lawrence  -> Fight Complexity with Complexity

March 08, 2004 - Fight Complexity with Complexity

We were using method and class and package names as keys to the various data structures that store test results and coverage data. Agitator told us that if you pass the string "123%%^*abc" to a method that expects a class name, it throws an IllegalArgumentException. "Well, duh!" we said and marked it expected. We added a factory to generate a variety of good and bad class names and got on with the task at hand.

The same thing happened in a bunch of different places, but it was no big deal. The code to validate a class name was very simple and Agitator made it easy to re-use the factory. We scanned the agitation results and it looked liked we had forgotten to handle the default package. The logic to deal with it was simple and we only had to change the code in a few places.

About a week later we were adding the code to accumulate the coverage results and we noticed some duplication. We had four methods - for project, package, class and method - that were almost exactly the same and it wasn't immediately obvious how to eliminate the duplication.

A Whole New Type

We could add a new type to represent each scope but it felt like overkill to add a whole new class just to represent the name of something. It would only eliminate 5 lines of code that were repeated 4 times. Against our better judgement, we gave it a try anyway. We could always back it out.

We added a class that we tentatively named ScopeKey and changed all the method signatures to take a ScopeKey instead of a project/package/class/method name (31 of them) for a net saving of a hundred lines of code or so. "Wow !" We had no idea we used it so often. All the tests passed.

This was our first Agitator-induced refactoring. We hadn't realized it at first. Agitator was telling us something was wrong, but we thought the answer was to make the test more sophisticated. It would have been easier to make code simpler.

We didn't need to assign the class name factory to every method any more. We assigned it once to ScopeKey and Agitator used it whenever it needed a ScopeKey. All those IllArgs went away and we felt good.

Actually, they didn't all go away. Agitator said that you get an IllegalArgumentException when you give a package name to a method that expects a class name. "Well, duh!" we said and marked it expected.

The Class Gets Bigger

Now that we had a type to represent class names, it made sense to pull all of the class utility functions into one place. You know the ones I mean : All those static functions in an abstract class called XXXUtil with a private constructor because it's overkill to create a whole new object just to get the simple class name from a fully qualified name.

We did the same for the code that finds the parameter names given a method signature. This code had been hard to test because it was in the middle of the method that needed the parameters... actually, it was in the middle of more than one method but that's a different story ... but now we could agitate that code. Agitator found the bug that occurs when the second to last parameter is a primitive type.

By now our little class that just held a string was getting bigger and we noticed that all of the methods had the same nested if-else ladder : if it's a class, do this; if it's a method, do that etc. "Replace Conditional With Polymorphism", we thought - but that was definitely overkill. "A whole class hierarchy ? Just to hold a string ?"

One Class Turns into Four

Agitator told us that a ScopeKey that holds a class name blows up when you ask for it's parameter names and that "‹default›" is not a valid method name. By the time we reached the fourth such problem that polymorphism was sounding more attractive.

We went ahead and did it and you know the ending by now. We deleted a bunch more code, all our IllegalArguments went away, the code was easier to read and Agitator was able to give us much better observations about our code because it knew not to pass a package name to a method that expected a class name. Less noise. Better observations. More reuse of test assets.

All the time we thought of a class name as just a string, it had no behavior. Once we created a type for it - once we reified it - we realized that class names had a lot of rules and behavior that had previously been distributed throughout the codebase. We had added a little complexity and were rewarded with less complexity overall.

We are learning to listen to the Agitator more now - and also finding ways to make the Agitator speak more clearly when something needs attention. Next time, we'll just introduce the new type in the first place.


Posted by Kevin Lawrence at March 8, 2004 05:17 PM


Trackback Pings

TrackBack URL for this entry:
http://www.developertesting.com/mt/mt-tb.cgi/114


Comments

Post a comment




Remember Me?