The colours thing bites

A while back I found what might politely be called “a mixing of business logic with display logic” issue in the refactoring project. Yesterday it bit me…

So we have an object that has a set of named properties. These properties are a mix of business logic and the display logic for 2 or maybe 3 distinct display modes. That’s bad enough in itself but some of the object status values seem to be represented only as a colour… Several distinct object states map to a single value that stores a single colour value. It seems that the order in which the states are tested for determines the outcome of the colour… Interesting design choice.

It could be worse. No, actually it is worse. This colour is the text colour in the display grid that’s displaying the object. The various object status values aren’t explicitly stored in the object at all.

So this week’s simple fix was “display this property in this colour when this happens”. That’s easy. The corresponding bug fix request after that release was “when we fix the cause of the problem, change the colour back to what it was”.

Suffice to say, refactoring the display logic is quite high on the list, but it’s a big job that requires thought and it’s not scheduled for now so we need to fix the problem by working around the interesting design rather than redesigning our way out of the hole.

Recap: To work out the colour that a property is displayed in requires doing complex things to the object in the correct order and these complex things are spread all over the place (someone had a real “wherever I lay my cursor that’s where the code goes” attitude to responsibility allocation). Changing the property that we need to recolour doesn’t cause the whole “recalc the object” thing to happen so we can’t just rely on the property being the right colour unless we decide to paint it our error colour…

The eventual “fix” was a nasty hack. I didn’t expect it to be otherwise really. When the original design is so broken each change tends to end up being one nasty hack on top of another. The code gets worse, the design gets worse… Did someone at the back say “death spiral”?

We now have a map of saved colours. Before we change the colour of the property to signal the new error state we push the current colour into the map. If we then fix the problem we look in the map to see if there’s a saved colour and we restore it… Nasty, nasty, nasty.

I’m looking forward to the day when this object finally has its display logic untangled from its business logic. Lots of complexity and bandage hacks will go away. It’s on the list, but then so are lots of other things…