If broken it isn't, fix it you shouldn't...

From speaking to some old friends of mine I heard about a cataclysmic implosion of a project I used to work on. This was a project that initially started as an extremely badly coded asp.net application (with horrors like functions with 1000+ lines of code). A project that we spent over a year on trying to upgrade to use much better patterns and practices and add tonnes of test coverage on while changing/adding features to the application. It was by no means perfect when we brought it over the finishing line, but it was a very solid and decent bit of code. The damn thing even won an award!

Anyway, it led me on to writing this blog post. And my tip of the day is:

"If broken it isn't, fix it you shouldn't..."

The basic concept I put forward is if something isn't broken then why change it? If you're changing something just because it's not agile enough, or you don't like the code, then you're not really following agile principles, or at least not following one of my favourites: Take the first bullet. Some people call it "fool me once, shame on you, fool me twice, shame on me". Effectively, if a requirement or story "fools" us into not making that abstraction, or into writing code that doesn't protect us from that bug, then that's the first bullet. When we get hit by that bullet it's time to abstract away or protect that code so that we don't get hit again.

Here's a really basic example: Let's say I ask you to write some code to sort a list of contacts by their name. You might write me a sorter class that takes in the list of contacts and returns a list sorted by their name. You might even go as far as passing in a parameter so that you can sort it by any field you want.

Then comes a second requirement - I want to sort it by email address as well as name, because I have 20 joe bloggs. Ok, so we didn't account for the ability to sort by multiple fields. One solution to this problem is add a parameter to allow you to add a second field to sort by. But what if we get 3 fields? Or what if the new field didn't have the same way of sorting as name? A better solution would be to introduce Comparer objects or something of that manner. That way you let the invoker tell you how to compare the objects and that way you've taken the first bullet. What we have in effect done is followed up by closing off a possibility of further sorting issues. (For more information on this look up OCP, or Open/Closed principle).

This is exactly what didn't happen in this "cataclysmic" project. Things were deemed "not agile enough" so changes were being made to modules and pages and classes that weren't taking any "bullets". Next thing you know things stopped working, because the whole project architecture started to change and remould over time. This is probably very much expected. So this (every so slightly) changed the situation into "broken it is (now), so fix it you bloody well should!". Their decision? Rewrite the whole project. My jaw left a dent on the floor when I heard that bit, no word of a lie... Such madness!

Finally, if you're astute you may have recognised that my blog entry title was very much inspired by an excellent blog by Tess Ferrandez! (If broken it is, fix it you should)