The inner radar

A while ago, I was asked to review the quality of a software project. The technology was based on Java Enterprise Edition.

After the first look, I stumbled across a particular class that had some 5000 lines of code. The class was named something like XyzManager. Obviously, like almost any manager-named class I have seen, the class tried to do too many things. I immediately pointed this to the team, but they were not impressed. Of course, they knew about that class. In fact, it was hard to look at this project and miss it.

Their problem was that they did not know how to fix it. So, they challenged me to look into it and propose a way to refactor it.

While going through the code, I noticed that there seemed to be too many somewhat similar if statements. To check if they were indeed similar, I ran a duplication analysis only for the methods of this class, and I discovered that indeed, large chunks of code were duplicated multiple times, and that these pieces of code included long lists is if statements. At a closer inspection, all these blocks were checking for an instance variable called something like xyzType. The code looked something like:

if (xyzType=A_TYPE_VALUE)
  doSomethingForAType(...)
else if (xyzType=ANOTHER_TYPE_VALUE)
  doSomethingForAnotherType(…)
…

Essentially, they were missing a state pattern. Looking further into the domain, I concluded that what was needed was a mini-state machine.

I went back and told them about this. Based on the duplication detection, I could list both the classes that should form the State hierarchy (essentially based on the names of the strings they were checking for), and the methods that were needed in these classes (based on the code inside the if).

And, when they looked reasonably happy, I told them that their problem had nothing to do with the state pattern. "What do you mean?" they asked. "Well," I said, "your main problem is that someone opened the class, entered line number 5000, committed the code, and then slept well at night".

The alarm should have been triggered way before the problem got so prominent. But, it did not. The problem was that the team's inner radar was broken. In this situation, spending the refactoring effort might have solved the problem in the short term, but it was bound for a new problem to appear in some other shape.

The inner radar is an instrument that must be developed and protected, because it is the key to ensure the long term health of the project. It's like preventive medicine: it's not about medicines, it's about what we eat and do every day. You have to cultivate the state of mind through exercises that have no direct influence on a particular sickness, but that still maintain the overall health of the system.

Humane assessment is a means to get you towards this goal. On the obvious side, approaching software understanding problems in a smart way, through crafting custom tools, will get you towards better solutions. But, the key benefit comes from formulating the questions in the first place. This very exercise will get you to ask ever smarter and more introspective questions. This will ensure that the inner radar remains vigilant.

Posted by Tudor Girba at 18 February 2012, 11:51 pm link
|