Severance pay

We need to add a new feature to a severance pay software. Legacy software. Very legacy. Normally we would discuss the requirements, inspect the code, estimate, sign an agreement, develop, release and get the money. We do not even need more than one iteration, as the change is relatively simple and small. All is well until we reach the "inspect the code" part (yes, just the second step...).

The code is a nightmare in which side effects rule the world, and you're not even aware of it. As time goes by, and your understanding of the code increases ("understanding" is a real overstatement) you start to realize that democratically elected objects and methods are no longer in power: the dark times of side effects have come, good code long time forgotten. And you are doomed to become insane. Of course, being already on my way to insaneness, I have an undeserved advantage. But that's a whole different story.

Well, we definitely have to understand this code. After banging our heads on the wall for some time we decide that the first thing we'll do is to look for the side effects and turn them into methods which names tell you the story, so if a method sets the foo variable it is called setFoo and not getTheValueOfThatFieldCalculatedForThatPeriod. Easier said than done, because side effects have side effects, which in turn have side effects and so on. The FIRST thing we have to do is to build a test harness (don't tell me you thought for a single moment the code had tests). Thus, as everything that happens deep in the dungeons of the code is a strange mixture of lore, smoke and magic - and an insane amount of luck - we decide to test the final output of the whole calculation. Ok let's do it.

First: let's create some test data, decoupled from the database. After all, we don't want to waste unnecessary time hitting the database (and we should all refer to the same one). That's long but easy, as many values and properties are required. while (not done) {Clicketi clacketi tic tic clack click clack} ok we're done. Let's write a simple test and let's check the very last value that the output page would display. Clack click clack... Run test...

...Error: xxx.persistence.XxxPersistenceException: pkg1.pkg2.pkg3.CoefficientTable: no default connection found.

NO DEFAULT CONNECTION FOUND? Whatthe... ok, ok, the application reads the coefficients used in the calculations from a database. We make a note to decouple it later, maybe the application will load them during the startup, we do not care where they come from, what we really want is to inject the coefficient data when we actually need to make the calculations. For now let's use a TestSetup and connect to the database (and disconnect when we're done). Running JUnit tests...

1 test failed... junit.framework.ComparisonFailure

ComparisonFailure? But if I actually print the page I get the right result... and the property I check in the test is the one we use for the output... What...???

A-ha! That's where we discover that the software that calculates the final values does not really calculates the final values, despite the name of the methods, because the class that generates the HTML of the output page (why a class? why not a servlet? why not a simple jsp page and a bean?) makes even more calculations. Yep, you guessed, based on side effects as well. Undeclared and inconceivable side effects, of course.

I do not dare to ask my team to estimate the tasks for the new feature... I do not even dare to ask them to estimate the time they will need to be able to estimate the new feature...

Let's close our eyes, hold our noses and refuse to listen... any free wall? we've got some heads to bang... BUT NOT OURS! My team members want the scalp of the original developers... and who am I to prevent them to be happy?