Cykod Blog

Legacy spagetti code, aka "Bad code that works". It's a phrase can give a programmer shivers running down their spine and can be used to scare just about any new hire to do just about anything you need them to do

- Hey Billy - do you want to clean the toilets or fix that hack Phil put together for last year's demo?

Toilets. Every time (unless they have a masochistic streak, in which case nicely ask them to keep their leather and dog-collar activities to themselves)

As code goes I think you could roughly categorize four types in descending order of usefullness:

Good Code that works
Bad Code that works
Good Code that doesn't work
Bad Code that doesn't work

While some might argue that you can't have "Good code that doesn't work" - I believe you can. You can have a georgeous set of fantastically OOP'd classes that do B when the client wants A; Or spotlessly clean program flow that's nicely commented and happens to run too slow by a couple orders of magnitude. "Works" doesn't mean compile - it means gets the job done. What to do with Good code that doesn't work?
The same then that you do with Bad Code that doesn't work - rewrite, re-factor and redo what needs to be done to fullfill the requirements.

What do you do with Good Code that works? Pat the developer on the back and be respectful of the design. When it does need to be modified, make sure you have an overarching view of what eeds to be done and how it can done within the framework of the existing code. Don't turn it into legacy spaghetti code (or if you do - at least have the decency to put your initials up at the top so people will know who to curse silently under their breath without hunting through the SCM logs)

Now where there can be an issue is #2 - Bad Code that works. Yes, it's a jumbled pile of functions name foo, bar, and foobar - but it gets the job done and provided it's not touched, gets people home on time.

The most important thing is to overcome the intense impulse to rip the whole thing out while you do a complete rewrite from the ground up and apply every single one of the design patterns that you learned in the last book you read. Chances are if that code has been around for a while it's got a whole bunch of fixes for stuff that people didn't get right the first time around, and if you throw all of that would you'll be throwing out the baby with the bathwater. Any institutional knowledge embedded in that code will be lost when it goes out the window. A lot of what ends up in bad code is there for a reason - yes, sometimes that reason is just that programmer A had know idea what they were doing - but oftentime there are fixes for conditions that were overlooked the first time around that you need to make sure are incorporated into any refactoring. 

What you need to do in this situation is to take a step back, categorize the specific pieces of functionality that makes that code "work" (in whatever sense of the word is important to you or your organization), and then go ahead and write yourself some tests validating the current behavior. If the code already has a set of tests associated with it - all the better - but still take a look at those tests and try to up the coverage to as close to 100% as possible. Search through version control and ask around the user base if anyone remembers any problems that cropped up and were fixed. Those are the sort of features to validate and make sure you have regression tests to prevent them from happening again. 

Not set up with a testing framework? Now would as good a time to get one set up, as for an experienced developer I'd say the real value in testing is not in generating code, but rather in not breaking it with modifications down the road, as you're already down that road you'll see the benefits immediately.

There's plenty of frameworks out there - I'm a big fan of RSpec on the Rails front. But depending on the code, something as simple as a script that fetches program output and matches it against a couple of strings or regular expressions is still a whole lot better than nothing.

Once you have good code coverage on the module you are changing, refactor away - turn it into a shimmering piece of Good Code that works and earn yourself that pat on the back - I'm sure that fancy Factory-Delegation-Adapter pattern will do just the trick.