Writing clean code in a legacy codebase

Published in

PayU Latam Engineering

9 min readJul 20, 2020

There’s a false assumption about software that makes people think of a trade-off between quality and speed. Most people believe that we could drop quality to gain development speed. This argument is commonly held between management and development teams, and the saddest part is that most of the time, developers agree to this unrealistic trade-off.

There are many arguments to change this impression. But, in my opinion, the fact that low-quality software is more costly than high-quality software just after a few weeks, and the results observed in studies such as the State of DevOps Report are two ways to start the discussion. The following paragraphs will try to show just that.

As software developers, we know that with time the complexity of our software increases. But there are two types of complexity, the first one is inherent to the problem domain we are addressing, and the second one is the unintentional complexity added as a side effect of implementing features in our codebase. This second type of complexity is usually referred to as either technical debt or cruft.

The accumulation of technical debt increases the complexity of our software, and, since creating a new feature always require to start by understanding the current implementation, there’s an additional cost that needs to be paid just to begin writing code. Technical debt has various consequences but in this context, as suggested by Martin Fowler in is high quality software worth the cost? two are important to mention. First, it makes it harder to understand how to make a change. And second, it increases the chance to make mistakes (introduce bugs into production). Both of these facts lead to the same result, a slow down in the speed to create software.

As it should sound logical now, the only way to avoid the slow down in software development is to keep cruft to a minimum, so that teams can add new features with less effort. This fact is clearly stated in the State of DevOps Report, where the results show that elite teams that apply good software practices are capable of deploying software multiple times per day, while in contrast, low performing teams take months to deploy a change.

I adhere to the thought that clean code is an important way to reduce the amount of technical debt in a codebase, and the rest of this article will try to exemplify that.

Why is clean code important?

The definition of clean code is somewhat imprecise and difficult. But, it’s rather easy to pin point bad code. You know when you are dealing with code that has lots of tangled dependencies and when changing something will break something else in an unexpected place (fragility).

In this article I refer to clean code as the programming good practices that make code more readable, flexible, and maintainable.

But, what does it mean for code to be “clean”?

To solve this question I am going to introduce a small example.

A bowling alley hires you to build their system, and you are tasked to start by implementing the scoring module.

Have you ever bowled before? (If you haven’t, take a look at this Wikipedia entry)

As the problem seems really trivial, you start coding right away:

Dirty implementation of the bowling alley

This implementation works as expected, and the customers and your boss are happy because the tedious scorecards are part of history now.

After a few months and due to the success of the bowling alley, your boss decides that he wants to make a small change to the scoring system. He wants to give 2 additional points every time you complete three strikes.

You think this seems trivial, and start hacking your way into your old code.

Dirty implementation of the bowling alley with bonus

By adding the str variable and doing two statements with it (and increment and using it to increase the out variable), you got what you wanted. After running some manual tests on your code a couple of times, you feel satisfied with the results.

A couple of months go by, and after more success with the changes, your boss decides that he now wants to add another change:

He wants to have a variable number of frames (not just 10 as in the basic game)

When you go back to check the code, you realize you are in trouble:

The variables’ names don’t mean anything
There’s a lot of nested if statements
Lots of 10 appear on the code, and you don’t remember which ones refer to the number of frames and which to the number of pins

Making changes now is risky. How can you now test that by adding the frames functionality you won’t break anything?

Moreover, since the first change was so fast, the bowling alley already advertised the change for next week.

After this, you must decide between two alternatives:

Attempting to understand the code, and modify it to add the new functionality to the game while running lots of manual tests, or
Add some unit tests to make sure you won’t break things. And also refactor the code to make it more readable and attempt the changes.

How to create clean code in a legacy codebase

The previous example illustrates the value of clean code to allow fast changes in a codebase. But when we are applying changes for software that has already been in production, we usually have to deal with difficulties such as the lack of knowledge of the expected behavior, lack of documentation, bad code practices, and all sorts of complexities. To this type of code, we usually refer to as Legacy Code.

I’m one of the people who have a hard time believing that the word legacy has a bad connotation. How can it be that our legacy is something people are going to complain about? — As uncle Bob puts it in his article code hoaders this usually happens because we put more effort into adding new features(regardless of how they are implemented) than organizing existing ones.

But if you’re already dealing with this situation, I think you can decide to do one of the following two things:

Keep adding complexity, applying poor practices and increasing the bad connotation of the word Legacy, or
Improve the existing codebase and be proud of your legacy (your code) in that project

If you want to do the second thing this is my advice to you:

Work having feedback

As Michael Feathers puts it in his book Working effectively with legacy code, there are two ways to make changes in a system:

Edit and Pray, or
Cover and Modify

Edit and Pray is much like working with care. That means, analyzing the existing code and trying to apply the changes as carefully as possible. But, working with care is not good enough if you don’t apply the right tools and techniques.

Cover and Modify is completely different. The central point is that you work with a safety net while modifying code. This safety net means that we must cover the code we are modifying with tests. This way, we can have fast feedback about the effect of our changes. There are different types of tests, but if we want agile feedback, we need tests that run fast and that you can run frequently (run it, every time you make a significant change, this must be less than a few minutes apart). This takes as to the concept of Unit Tests (basically, tests that run fast and help us localize errors).

The problem is that if our code doesn’t have tests, we must add them. But, to add tests you usually have to modify code. This is referred to as The Legacy Code Dilemma, and the solution is to perform safe minimal refactoring that allows us to add tests.

Adding tests can be a nightmare! Most of the time, the existing code was not written to be testable in the first place (and yes! testability is another characteristic you must have in your systems).

This rigidity usually comes from having hard dependencies on other objects. This can come in different flavors, but database connections are a common example.

This makes it very hard to mock the behavior of the repository and forces us to have a connection to Postgres every time we want to test something.

Finding points to insert changes

Michael Feathers defines a seam as a place to alter program behavior, without changing the code.

By introducing an Interface, and receiving the repository as an argument to the constructor, we open the place to alter the behavior of our BusinessLogic object. This simple change creates a seam in our code. It allows the creator of the object to choose what type of Repository should be used. This would allow us to test our code without using a database connection.

Testable business logic object

Making changes on a hurry

In an ideal world, whenever you have to deal with existing code that doesn’t have tests, you would start by adding them. But there are some situations when business pressure and deadlines are really tight, and spending a couple of days to test existing code is simply not possible. What to do then?

As suggested by Feathers, you could use the sprout technique:

Identify where you need to make your code change — find the insertion point.
Create a method call to your new functionality. Get a sense of what the final result will be. This new method will be your sprout.
Create your code somewhere else and add tests to it (preferably use TDD)

To show this technique, I will use Feathers’ example.

Suppose you need to remove the duplication of entries before calling postDate(), but testing postEntries() is hard.

You could follow the sprout technique by creating a new method findUniqueEntries(). This new method can be fully tested and cleanly developed.

Feathers’ book has a lot more to offer in terms of strategies to deal with specific situations you’ll find when dealing with legacy code. I highly recommend anyone interested in the topic to take a look at it.

Feature flags as another safety net

Feature flags (also known as feature toggles) are another tool that can help to avoid problems when facing legacy code. They allow modifying the system behavior without changing code. Feature flagging allows developers to wrap features in an if/then statement to gain control over its release.

By wrapping a feature with a flag, it’s possible to isolate its effect on the system and to turn that flag on or off independent from a deployment. Flagging gives another safety net to the newly developed code, by separating rollout from deployment.

Flags seem easy to understand. But their complexity level can vary widely depending on the level of control you want to accomplish with them:

You could change their value for specific sets of users allowing you to accomplish canary releasing.
You could manually control the value for specific hours and verify the results (do experiments).
You could use them to control the permissions of specific users.
You could use them to control the operational aspects of the platform.

For the sake of our discussion, the important point here is that you can add another safety net after releasing your code by making smart use of feature toggles in your code.

Changing the Bowling Alley to a Cleaner Version

Covering our code with tests

We will start by creating our safety net before modifying the production code. This way, we can have fast feedback about the effect of our changes.

For our Bowling Alley, the following Unit Tests cover:

Normal game (including strikes and spares).
An additional bonus of two points every three strikes.

Refactoring to clean code

Now, with our safety net in place, we can start refactoring our existing code, to make it easier to follow. Among the changes shown below you can find:

Variables were renamed to something easier to read
Indentation level was reduced (fewer levels inside methods)
Specific validations were taken to their own methods

But, if passing around arguments among methods bothers you, or if having two indentation levels does, you can go even further:

Adding the new functionality to our code

Remember, we want to have a variable number of frames (received as input for our class).

With our new code, the change is trivial, we just have to replace our static NUMBER_OF_FRAMES, for a constructor argument:

For the sake of simplicity, I am not showing the new unit tests added to verify the new functionality (but if this was real, they should be there! … do not open the door to untested functionality).

This demonstrates how easy it was to perform improvements when you have a clean codebase to work on, and how a safety net can help you deal with the uncertainty of making changes.

As a final remark, remember: your code will be your legacy at some point. Be proud of the legacy you leave behind by using clean code.