Working with Legacy Code
Introduction
Working on a brand new project, that you have complete control over, tends to be the exception rather than the rule. Most software developers will spend a significant portion of their careers working with legacy code. Existing systems that are already in production, have been around for years, and have been worked on by many different developers. This code is often complex, difficult to understand, and hard to change. Working with it requires a different set of skills than working on a greenfield project.
What is Legacy Code?
The term legacy code is hard to precisely define. Each developer you ask will likely have a slightly different definition. Some might use it to describe old codebases that are no longer maintained. Others might use it to describe code that is difficult to understand or change. I've even heard developers use to to describe any code that they didn't write themselves.
One of the most well known definitions comes from Michael Feathers in his book Working Effectively with Legacy Code.
"Legacy code is code without tests."
-- Michael Feathers
While this definition is catchy, it doesn't really capture the full picture. So, why does Feathers define it this way? Because code without tests is difficult to change without breaking something. Without tests, it is hard to know if your changes have unintended consequences. Tests give you a safety net that allows you to make changes with confidence, and they can act as a starting point for understanding the code.
The problem with this definition is that just having tests doesn't mean that code is easy to work with. You can have a codebase with 100% test coverage that is still a nightmare to work with. It might be overly complex, have poor naming, or be tightly coupled. There are even cases where having tests can make code harder to work with. For example, if the tests are slow, flaky, hard to understand, or so tightly coupled to the implementation that they need to be updated every time the code changes. So, while tests are a good indicator of how easy code is to work with, they are not the only factor.
I prefer to define Legacy code as:
Code that you are afraid to change.
The source of that fear doesn't really matter. It could be because the code is poorly written, because it has no tests, because it is poorly documented, because it is overly complex, or because it is critical to the business. The point is that you are afraid to change it because you are afraid of breaking something.
Safely Making Changes
No matter how messy some legacy system is, you need to remember that it is currently providing some value to the business. It is running in production, and it is doing something useful. So, when you are working with legacy code, you need to be very cautious. You need to ensure that existing functionality is not impacted by your changes, and that new functionality is added correctly.
"Software engineering is immune to most physical laws, but we get hit hard by entropy."
-- Andrew Hunt and David Thomas, The Pragmatic Programmer
Most older code is terrible. Complexity creeps in through a series of small changes, each of which makes sense in isolation. Rotating developers, changing requirements, and shifting business priorities all contribute to the mess. The longer a system is around, the more complex it becomes, regardless of the intentions of the original developers.
The code you write today is the legacy code of tomorrow.
This isn't incentive to discard best practices, quite the opposite. It is a reminder that the decisions you make today will impact the developers who come after you. Seeing your work become legacy code should be something you're proud of. It means that you've built something that has stood the test of time.
Alleviating Fear
If you are going to be regularly working in a legacy codebase, you need to find ways to alleviate your fear of making changes. The steps required will depend on the codebase, and the reasons you are afraid to change it. We will rarely have time to completely refactor a legacy codebase, so we need to find ways to make small, incremental changes that improve the codebase over time.
Gaining Familiarity
The most common reason newer developers are afraid to change code is because they don't understand it. The first step to alleviating this fear is to gain a deeper understanding of the codebase. This means reading through the code, running it, debugging it, and making small changes. You need to understand how the different parts of the system interact, and how changes in one part of the system can impact other parts.
Some useful techniques for gaining familiarity with a codebase include:
- Read the Code: Start by reading through sections of the code. This can be a slow process, but it is essential to understanding how the system works. Pay attention to the high-level architecture, the major components, and the interactions between them. Start by focusing on a single workflow or feature, and trace it through.
- Fix Minor Issues: Make small, safe changes to the codebase to understand how it works. These tiny, safe, and reversible changes can help you understand the codebase without the fear of breaking something. They also build some comfort in preparation for larger changes. This could be fixing typos, renaming variables, or removing dead code. Just make sure it is something that can be easily undone.
- Document as You Go: As you learn about the codebase, write down what you learn. Ideally somewhere that is part of the codebase and accessible to other developers. This will help you remember what you've learned, and make it easier for other developers to learn from your experience. Sequence diagrams, class diagrams, and flowcharts can be helpful here.
- Scratch Refactorings: Just tear stuff apart. Make a copy of a class, and start ripping out methods, changing names, and moving things around. This is a great way to learn how the codebase works, and to see how changes can impact the system. Just make sure you don't check these changes in. Throw them away when you are done.
Write Tests
Because a lack of tests is a common definition of legacy code, there is a temptation to just jump in and start writing unit tests. However, this is often a waste of time. Tests that are written after the fact are hard to write and usually tightly coupled to the implementation. Instead of making the code easier to modify, they can make it harder. You now need to update the tests as you refactor the code, which can be a significant amount of work.
Unit tests can still be helpful, but not to the same extent as they are in greenfield projects. In a greenfield project they act as a vise, ensuring that changes don't break existing functionality. In a legacy codebase, you should treat them as an exploration tool. They let you verify your understanding of how the system works.
Trying to pad your test coverage numbers is a waste of time. Instead, focus on writing tests that help you understand the codebase. This could be tests that verify the behavior of a single class, or tests that verify the behavior of a single workflow. The goal should be to build up a suite of tests that give you confidence in your understanding of the codebase.
The best type of tests to write in a legacy codebase are approval tests. These tests capture the current behavior of the system, and allow you to make changes with confidence. They are especially useful when you are making changes to a system that is not well understood. Approval tests can be used to capture the output of a function, and then verify that the output doesn't change when you make changes to the function.
Adding New Features or Fixing Bugs
Most of the time, when you need to make a change to a legacy codebase, it is because you need to add a new feature or fix a bug. Due to tight deadlines, you are unlikely to have time for any larger refactor work. So, how do you make these changes safely?
You want your changes to be thoroughly tested through automation tests, but can't add tests for the entire workflow. The best approach is to isolate your new code from the existing codebase as much as possible.
Seams
The key to isolating your new code from the existing codebase is to find seams. The term seam comes from Michael Feathers' book Working Effectively with Legacy Code.
"A seam is a place where you can alter behavior in your program without editing in that place."
-- Michael Feathers
In other words, you can insert code at a seam to change the behavior of the program without changing the code around the insertion point. Seams allow you to isolate the new code from the existing code, and to test the new code in isolation. This makes it easier to add new features without breaking existing functionality.
Most seams are found at the boundaries of the system components. This could be a function call, a class instantiation, or an internal dependency.
The excellent examples for this section are based on this article by Nicholas Carlo, which offers a concise summary of Michael Feathers' book Working Effectively with Legacy Code.
When adding a new feature to an existing system, you have two main options: the sprout method and the wrap method. With the sprout method, you add a call to a new function that contains the new behavior. With the wrap method, you add a wrapper around the current function that contains the new behavior.
Sprout Method
Using the same example for both wrap and sprout, here's our starting code. Let's say that you have a function called postEntries that is not supported with good tests, and you want to eliminate duplicate entries in the input. Ideally, we'd add thorough test for postEntries before we start, but it's unlikely that we have time for that. Here's our starting code:
class TransactionGate(private val transactionBundle: TransactionBundle) {
// ... a lot of code here
fun postEntries(entries: List<Entry>) {
entries.forEach { entry ->
entry.postDate()
}
// ... a lot more code here
transactionBundle.listManager.addAll(entries)
}
// ... even more code here
}
- Create a new function that contains the new behavior some place outside of where you need the functionality.
- Test the new function thoroughly in isolation.
- Add a call to the new function where you need the functionality.
Create a new function uniqueEntries that eliminates duplicates. Once finished we can thoroughly test it in isolation.
Then, add a call to uniqueEntries in postEntries. The highlighted lines have been changed from the original code.
class TransactionGate(private val transactionBundle: TransactionBundle) {
// ... a lot of code here
fun postEntries(entries: List<Entry>) {
val uniqueEntries = uniqueEntries(entries)
uniqueEntries.forEach { entry ->
entry.postDate()
}
// ... a lot more code here
transactionBundle.listManager.addAll(uniqueEntries)
}
// ... even more code here
}
Wrap Method
- Rename the existing function to a new name.
- Create a new function with the old name and signature that calls the renamed function.
- Add the new behavior to the new function. This can be before or after the call to the renamed function.
class TransactionGate(private val transactionBundle: TransactionBundle) {
// ... a lot of code here
fun postEntries(entries: List<Entry>) {
// Do some clever logic to dedupe entries
postUniqueEntries(entries)
}
private fun postUniqueEntries(entries: List<Entry>) {
entries.forEach { entry ->
entry.postDate()
}
// ... a lot more code here
transactionBundle.listManager.addAll(entries)
}
// ... even more code here
}
To test this work we would need to mock the call to postUniqueEntries and verify that it is called with the correct arguments.
Larger Scale Refactoring
If you are working with a legacy codebase that is still under active development, it may be worthwhile to invest in some larger scale refactoring. This could be to improve the overall structure of the codebase, to remove technical debt, or to improve the performance of the system. However, this is a risky proposition. You need to be very careful when making large scale changes to a legacy codebase.
Find the Pain Points
The first step with any large scale refactoring effort is to decide what you want to change. A complete rewrite is rarely a good idea. Instead, you should focus on the parts of the system that are causing the most pain. Pain, in this context, typically refers to areas that are causing the most bugs, slowing down development, or causing the most customer complaints.
These are not always the most complex areas of the system. A complex region of code that is rarely touched might not be causing any pain. Instead, focus on the parts of the system that are actively causing problems.
Some ways to identify pain points include:
- Bugs: Look at the bug tracker to see which parts of the system are causing the most defects. If the same region of the code is being updated during bug fixes, then that region is likely a pain point.
- Talk to Developers: Ask the developers who work on the system what parts of the codebase they find the most difficult to work with. They will likely have a good idea of where the pain points are.
- Review Commit History: As long as your team is using a consistent process, the repository can be an excellent resource. Look at the commit history to look for files that are modified more often than the rest of the system. If the same region of the code is being updated frequently, then that region is likely a pain point. You should also look for commits that are reverting changes, groups of files that are modified together, or commits that are touching a large number of files.
Using a combination of these techniques, you should be able to identify the parts of the system that are causing the most problems for users and developers.
Add Tests
Once you have identified a candidate region of the codebase for refactoring, you need to capture the current behavior. Don't concern yourself with whether the system is behaving correctly, just capture the current behavior. Since the system has been in production for some time, you can assume that the current behavior is correct. We just want to ensure that we don't break anything when we make changes.
Add tests that interact with the candidate region through its public interface, and capture the output by recording it. This could be through logging, database queries, or other side effects. Use test doubles to isolate the region from the rest of the system. This could be through mocks, stubs, or fakes depending on the situation.
Refactor
Once the tests are in place, you can start refactoring the code. The goal of the refactoring should be to make the code easier to understand, easier to change, and less error-prone. This could involve breaking up large classes, extracting methods, renaming variables, or removing dead code. Tear the code apart and put it back together in a way that makes sense.
As you refactor the code, run the tests to ensure that you haven't broken anything. If you do break something, you can use the tests to identify the problem and fix it. I suggest running the tests after every small change, and committing the changes to version control. This way, if you do break something, you can easily roll back the changes.
Conclusion
Working with complex legacy systems requires a different approach than working on greenfield projects. You are working with a system that is already in production, and that is providing value to the business. Any changes you make need to preserve that value, and ideally add to it. This requires a cautious approach, and a focus on making small, incremental changes that improve the codebase over time.
When larger changes are required, you need to start by adding a safety net of tests. These tests allow you to make changes with confidence, and to ensure that you don't break anything. Once the tests are in place, you can start refactoring the code to make it easier to understand, easier to change, and less error-prone.