Friday, August 14, 2009

Beany Objects and Busy Code-Behinds


My new partner and I had a task which involved a new feature. The complication is that the current version of the feature was implemented almost entirely in the initialization method of a web page code-behind class.

Yeah. The method that we wanted to test in Fitnesse was almost entirely written into the GUI, and worked entirely in terms of the query it was building to populate the screen. This was not welcome news, because the Fitnesse test couldn't make use of the UI Screen's code.

Sadness is that this situation isn't all that uncommon.

In my last company, there was code that implemented the same feature two different places: once in the web UI and again in the report generator. Of course, changes to the one would not affect the other. The code had two different maintenance points, and each was written in an idiosyncratic way to thread the calculations into the screen or report. A customer was simply flabbergasted that the developers had written the same functionality two different ways, in two different places. I shared his surprise. If this had been a single class or method, it could have been used by the screen and the report and the unit tests and the fitnesse tests.

But now I am a little saddened. There are definitely parts of the UI that are big, busy UI code-behind methods working on big, stupid, beany classes. That means that we are very likely to have duplicated code and the usual abuses of program-craft.

We've known for years that the right place for functionality is in functions. We know that objects exist as places to hold functions that are relevant to a single area of responsibility. This is the golden rule of coupling and cohesion:
Place things that belong together together.

The second rule is like it, and applies to OO coupling/cohesion:
A class has a single responsiblity. It does it all, does it well, and does it only.

And the final rule, the most practical and pragmatic of all, slams to door on so many abuses:
Code is written Once And Only Once (no duplication!).

If we violate the first two rules, we necessarily violate the final rule, or cause it to be violated by our peers. If they cannot find or cannot separate the code they want from the code we've written it into, then the easiest thing for them to do is to duplicate the code without the unrelated concerns we've woven into it.

I personally have a "moment of decision" when I find that the code I want is in lines 5-8, 12, 30-35, 40 and 78 of a 100-line function. I know that the right thing is to extract that method into a the appropriate class (creating such a class if it does not exist), but it sure seems easier to avoid breaking that long function by rewriting the code elsewhere. These moments of decision are warning signs.

This method broke my heart a little. Its job is to initialize the screen, but it does this in the course of a very long method that combines magic and sql and object method calls and many responsibilities woven into its tangled web of multiple-responsibilities and "convenient temporal coupling". I bet it wasn't that easy to write, either. You can bet it is untested. And you know that this week we're going to have to wrap tests around it and break it into pieces, and reorganize pieces into classes, and wrap those classes in tests as well. In the end, it should be better. Sadly, it is likely that it will stretch our feature completion date out by a few days.

If we didn't have to rewrite badly-coupled code, we could be truly done much more quickly.

We could "seem" done by ignoring the ugly code now, but that just shovels the risk and mess onto the next pair of programmers who stumble into this area. It's possible that they'll be in the mess because there were no tests to detect some breakage we caused. It will hurt them later or it will hurt our schedule now.

Leaving broken and ugly code is not only unprofessional, it is also irresponsible, and cowardly. It would say that we value our own time more than our teammates' time, and more than we value the quality of our product. I'll go "late" for this reason. It has to be done, and I like to sleep at night.