Chapter 7. Wallowing in Filth

As a dog returns to its vomit, so fools repeat their folly.
Psalms 26:11

We’ve all encountered it: quicksand code. You wade into it unawares, and pretty soon you get that sinking feeling. The code is dense, not malleable, and resists any effort made to move it. The more effort you put in, the deeper you get sucked in. It’s the man-trap of the digital age.

How does the effective programmer approach code that is, to be polite, not so great? What are our strategies for coping with crap?

Don’t panic, don your sand-proof trousers, and we’ll wade in…

Smell the Signs

Some code is great, like fine art, or well-crafted poetry. It has discernible structure, recognisable cadences, well-paced meter, and a coherence and beauty that make it enjoyable to read and a pleasure to work with.

But, sadly, that is not always the case.

Some code is messy and unstructured: a slalom of gotos that hide any semblance of algorithm. Some is hard to read: with poor layout and shabby naming. Some code is cursed with an unnecessarily rigid structure: nasty coupling and poor cohesion. Some code has poor factoring: entwining UI code with low-level logic. Some code is riddled with duplication: making the project larger and more complex than it need be, whilst harbouring the exact same bug many times over. Some code commits “OO abuse”: inheriting, for all the wrong reasons, tightly associating parts of code that have no real need to be bound. Some code sits like a pernicious cuckoo in the nest: C# written in the style of JavaScript.

Some code has even more insidious badness: brittle behaviour where a change in one place causes a seemingly unconnected module to fail—the very definition of code chaos theory. Some code suffers from poor threading behaviour: employing inappropriate thread primitives or exercising a total lack of understanding of the safe concurrent use of resources. This problem can be very hard to spot, reproduce, and diagnose, as it manifests so intermittently.

(I know I shouldn’t moan, but sometimes I swear that programmers shouldn’t be allowed to type the word thread without first obtaining a license to wield such a dangerous weapon.)

Key

Be prepared to encounter bad code. Fill your toolbox with sharp tools to deal with it.

To work effectively with alien code, you need to able to quickly spot these kinds of problems, and understand how to respond.

Wading into the Cesspit

The first step is to take a realistic survey of the coding crime scene. You arrive at the shores of new code. What are you wading into?

The code may have been given to you with a pre-attached stigma. No one wants to touch it because they know it’s foul. Some quicksand code you discover yourself when you feel yourself sinking.

It’s all too easy to pick up new code and dismiss it because it’s not written in the style you’d prefer. Is it really dire work? Is it truly quicksand code, or is it merely unfamiliar? Don’t make snap judgments about the code, or the authors who produced it, until you’ve spent some time investigating.

Take care not to make this personal.

Understand that few people set out to write shoddy code. Some filthy code was simply written by a less capable programmer. Or by a capable programmer on a bad day. Once you learn a new technique or pick up a team’s preferred idiom, code that seemed perfectly fine a month ago is an embarrassing mess now and requires refactoring.

You can’t expect any code, even your own, to be perfect.

Key

Silence the feeling of revulsion when you encounter “bad” code. Instead, look for ways to practically improve it.

The Survey Says…

We’ve already looked at techniques for navigating a new codebase in Chapter 6.

As you build a mental model of a new piece of code, you can begin to gauge its quality using benchmarks like:

Are the external APIs clean and sensible?

Are the types used well chosen, and well named?

Is the code layout neat and consistent? (Whilst this is certainly not a guarantee of underlying code quality, I do find that inconsistent, messy code tends also to be poorly structured and hard to work with. Programmers who aim for high-quality, malleable code also tend to care about clean, clear presentation. But don’t base your judgment on presentation alone.)

Is the structure of cooperating objects simple and clear to see? Or does control flow unpredictably around the codebase?

Can you easily determine where to find the code that produces a certain effect?

It can be hard to perform this initial survey. Maybe you don’t know the technology involved, or the problem domain. You may not be familiar with coding style.

Consider employing software archaeology in your survey: mine your revision control system logs for hints about the quality. Determine: how old is this code? How old is it in relation to the entire project? How many people have worked on it over time? When was it last changed? Are any recent contributors still working on the project? Can you ask them for information about the code? How many bugs have been found and fixed in this area? Many bugfixes centered here indicates that the code is poor.

Working in the Sandpit

You’ve identified quicksand code, and you are now on the alert. You need a sound strategy to work with it.

What is the appropriate plan of attack?

Should you repair the bad code?
Should you perform the minimal adjustment necessary to solve your current problem, and then run away?
Should you cut out the necrotic code and replace it with new, better work?

Gaze into your crystal ball. Often the right answer will be informed by your future plans. How long will you be working with this section of code? Knowing that you will be pitching camp and working here for a while influences the amount of investment you’ll put in. Don’t attempt a sweeping rewrite if you haven’t the time.

Also, consider how frequently this code has been modified up to now. Financial advisors will tell you that past performance is not an indicator of future results. But often it is. Invest your time wisely. This code might be unpleasant, but if it has been working adequately for years without tinkering, it is probably inappropriate to “tidy it up” now, especially if you’re unlikely to need to make many more changes in the future.

Key

Pick your battles. Consider carefully whether you should invest time and effort in “tidying up” bad code. It may be pragmatic to leave it alone right now.

If you determine that it is not appropriate to embark on a massive code rework right now, that doesn’t mean you are necessarily left to drift in a sea of sewage. You can wrestle back some control of the code by cleaning progressively.

Cleaning Up Messes

Whether you’re digging in for the long haul, or just making a simple fix-and-run, heed Robert Martin’s advice and follow the “the Boy Scout Rule”: Always leave the campground cleaner than you found it. It might not be appropriate to make a sweeping improvement today, but that doesn’t mean you can’t make the world a slightly less awful place.

Key

Follow the Boy Scout Rule. Whenever you touch some code leave it better than you found it.

This can be a simple change: address inconstant layout, correct a misleading variable name, simplify a complex conditional clause, or split a long method into smaller, well-named sub-functions.

If you regularly visit a section of code, and each time leave it slightly better than it was, then before long you’ll wind up with something that might be classified as good.

Making Adjustments

The single most important advice when working with messy code is this:

Key

Make code changes slowly, and carefully. Make one change at a time.

This is so important that I’d like you to stop, go back, and read it again.

There are many practical ways to follow this advice. Specifically:

Do not change code layout whilst adjusting functionality. Tidy up the layout, if you must. Then commit your code. Only then make functional changes. (However, it’s preferable to preserve the existing layout unless it’s so bad that it gets in the way.)

Do everything you can to ensure that your “tidying” preserves existing behaviour. Use trusted automated tools, or (if they are not available) review and inspect your changes carefully; get extra sets of eyeballs on it. This is the prime directive of refactoring: the well-known set of techniques for improving code structure.

This goal can only be reached effectively if the code is wrapped in a sound set of unit tests. It is likely that messy code will not have any tests in place, so consider whether you should first write some tests to capture important code behaviour.

Adjust the APIs that wrap the code without directly modifying the internal logic. Correct naming, parameter types, and ordering; generally introduce consistency. Perhaps introduce a new outer interface—the interface you wish that code had. Implement it in terms of the existing API. Then at a later date you can rework the code behind that interface.

Have courage in your ability to change the code. You have a safety net: source control. If you make a mistake, you can always go back in time and try again. It’s probably not wasted effort, as you will have learnt about the code and its adaptability in doing so.

Sometimes it is worth boldly ripping out code in order to replace it. Badly maintained code that has seen no tidying or refactoring can be too painful and hard to correct piecemeal. There is an inherent danger in replacing code wholesale, though: the unreadable mess of special cases might be like that for a reason. Each bodge and code hack encodes an important piece of functionality that has been uncovered through bitter experience. Ignore these subtle behaviours at your peril.

An excellent book that deals with making appropriate changes in quicksand code is Micheal Feathers’ Working Effectively with Legacy Code.footnote::[Michael Feathers, Working Effectively with Legacy Code (Upper Saddle River, NJ: Prentice Hall, 2004).] In it, he describes sound techniques to introduce seams into the code—places where you can introduce test points and most safely introduce sanity.

War Story: The Curious Case of the Container Code

There was a container class. It was central to our project. Internally, it was foul. The API stank, too. The original coder had worked hard to wreak code mischief. The bugs in it were hidden by the already confusing behaviour. Indeed, the confusing behaviour was a bug itself.

One of our programmers, a highly skilled developer, tried to refactor and repair this container. He kept the external interface intact, and improved many internal qualities: the correctness of the methods, the buggy object lifetime behaviour, performance, and code elegance.

He took out nasty, ugly, simplistic, stupid code and replaced it with the polar opposite. But in his effort to maintain the old API, this new version was internally far too contrived, more like a science project than useful code. It was hard to work with. Although it succinctly expressed the old (bizarre) behaviour, there was no room for extension.

We struggled to work with this new version, too. It had been a wasted effort.

Later on, another developer simplified the way we used the container: removing the weirder requirements, therefore simplifying the API. This was a relatively simple adjustment to the project. Inside the container, we removed swaths of code. The class was simpler, smaller, and easier to verify.

Sometimes you have to think laterally to see the right improvement.

Bad Code? Bad Programmers?

Yes, it’s frustrating to be slowed down by bad code. The effective programmer does not only deal well with the bad code, but also with the people that wrote it. It is not helpful to apportion blame for code problems. People don’t tend to purposefully write drivel.

Key

There is no need to apportion blame for “bad” code.

Perhaps the original author didn’t understand the utility of code refactoring, or see a way to express the logic neatly. It’s just as likely there are other similar things you do not yet understand. Perhaps they felt under pressure to work fast and had to cut corners (believing the lie that it helps you get there faster; it rarely does).

But of course, you know better.

If you can, enjoy the chance to tidy. It can be very rewarding to bring structure and sanity to a mess. Rather than see it as a tedious exercise, look at it as a chance to introduce higher quality.

Treat it as a lesson. Learn. How will you avoid repeating these same coding mistakes yourself?

Check your attitude as you make improvements. You might think that you know better than the original author. But do you always know better?

I’ve seen this story play out many times: a junior programmer “fixed” a more experienced programmer’s work, with the commit message “refactored the code to look neater.” The code indeed looked neater. But he had removed important functionality. The original author later reverted the change with the commit message: “refactored code back to working.”

Questions . Why does code frequently get so messy? . How can we avoid this from happening in first place? Can we? . What are the advantages of making layout changes separately from code changes? . How many times have you been confronted with distasteful code? How often was this code really dire, rather than “not to your taste”?

Example 7-1.

Employ the Boy Scout Rule. Leave every piece of code you touch better, if even only fractionally.