Legacy code is an unfortunate thing for any software developer to inherit. It's something many of us have worried about in our careers, and will probably continue to worry about. Throughout this post, we'll understand what legacy code is, what causes it, and how we can work to avoid it. Doing so will help us create a better developer experience for ourselves, and our future teammates.
This is an adaptation of a presentation I gave at Android Summit 2020. You can find the slides here.
What Is Legacy Code?
There are many different ways to define and interpret legacy code, so let's define a baseline understanding of what we mean for the intents and purposes of this post.
- Code that was written before we knew better - new tools and practices exist now.
- Code that was written by someone else who left our team - taking institutional knowledge with them when they left.
- Code that doesn't have any unit test coverage.
- Code that we're unable to/afraid to change because we don't know what might break.
- Code without any type of documentation or comments.
- All of the above!
None of these bullet points are mutually exclusive. When we refer to legacy code, it could be code that is one or all of these things.
Why Do We Care?
Why are we even taking time to discuss legacy code today? We need to understand legacy code because it impacts our work. Let's discuss how it does that.
Legacy Code Can Be A Blocker For New Development
If you have code that no one understands, or is tightly coupled to some third party tool, and you need to make changes or build on top of it - it's hard to do so! Which means you have to have a difficult conversation with your project manager about whether or not you can implement a new feature.
Legacy Code Can Prevent Us From Shipping With Confidence
If code is old, unclear, undocumented, and untested it will be hard to sleep at night when we ship changes to this code. If we can't have confidence in the changes we make, all releases will be more stressful than they otherwise would be.
Legacy Code Is Not Fun To Work With
For these and many other reasons, legacy code is just not fun to work with. Maybe you have a piece of code in your project that whenever you're asked to do development related to it, you automatically get nervous. This shouldn't happen! Code can be difficult, but we want this to be a pleasant experience as much as we can. So studying legacy code and avoiding it can help us create that pleasant experience.
Exploring Causes Of Legacy Code
Now that we understand what it is, and the problems it creates for us, let's look at some causes of legacy code.
Legacy Code Doesn't Have Tests
If you're not writing sufficient unit tests for your code, it is bound to become legacy code. This may not seem intuitive at first - we could have amazing code that is simply untested. However, as Jorge Coca points out, untested code becomes code that we're afraid to change:
Jorge Coca🐶 💙🦄@jcocaramos
@AdamMc331 If you have to work with code that you did not write and is considered a candidate for a refactor, not having tests makes the task incredibly complex and error-prone... however, if tests are present, then you have a security net to help you validate some assumptions16:46 PM - 07 Oct 2020
There are ultimately two reasons that code doesn't have tests.
- The developer chose not to write any - this can happen for a number of reasons.
- The code wasn't testable in the first place - making writing tests impossible.
The latter is a big red flag for other legacy code concerns, so let's look at what this means.
Untestable Code
Untestable code is usually code that has a lot of static references. References to global singletons that exist outside of our project that could be manipulating some global state that our tests aren't prepared to handle.
It can also refer to code that doesn't leverage proper dependency injection, meaning our class relies on dependencies that are hard to mock inside of our unit tests. This means we will have difficulty writing tests that can truly run in isolation.
Let's consider this example of untestable code. We've got a ProfileViewModel that tries to load a user's profile information, and if the network request fails, it will log that information in our Firebase dashboard.
class ProfileViewModel : ViewModel() {
private fun loadProfile() {
try {
// ...
} catch (e: Throwable) {
Firebase.getInstance().logException(e)
}
}
}
Please keep in mind this issue is not specific to Firebase, but we've chosen to reference a common error reporting tool to help readability.
There's a few reasons this code is difficult to test:
-
Firebase.getInstance()is not a method we have control over, so it is difficult to provide a mock implementation of Firebase for our unit tests. - We don't want our negative tests to actually record errors to our production Firebase tool. That would just create noise in our dashboard.
- The Firebase setup could be doing some work that our unit tests aren't prepared to handle, and could end up crashing our unit tests.
Untestable Code - Quick Solution
There is a quick solution to this problem, which is that instead of referencing a static Firebase instance, we can inject that instance into our ProfileViewModel using a constructor parameter:
class ProfileViewModel(
private val firebase: Firebase = Firebase.getInstance()
) : ViewModel() {
private fun loadProfile() {
try {
// ...
} catch (e: Throwable) {
firebase.logException(e)
}
}
}
This solves some of our testability problems. We now have the freedom to create our own mock Firebase implementation, and use that during our unit tests. However, this code is about to lead us into another common legacy code problem.
Tightly Coupled Dependencies Become Legacy Code
Companies change third party vendors all the time (different error reporting tools, different analytics trackers). They may also change tech stacks - REST APIs to GraphQL, for example.
Code that doesn't allow these things to change is likely to become legacy code, because it prevents those business decisions from happening easily.
Tightly Coupled Dependencies
Let's dissect this constructor parameter here:
class ProfileViewModel(
private val firebase: Firebase = Firebase.getInstance()
) : ViewModel()
Despite dependency injection, this class is still tightly coupled to the Firebase error reporting tool. This means that as Firebase changes, this class may need to change with it. If our company wants to move from Firebase to Sentry, or Embrace, we will have difficulty doing so because these tools may have completely different method signatures for logging errors.
Wrapping Dependencies
The solution to this problem is to wrap all of our third party dependencies. We do that by creating interfaces that define the expected behavior of the tools:
interface ErrorReporter {
fun reportError(error: Throwable)
}
We can then create our own implementation of this interface, that we can tightly couple to a third party tool:
class FirebaseErrorReporter : ErrorReporter {
override fun reportError(error: Throwable) {
Firebase.getInstance().logException(error)
}
}
As a result, we can update our ViewModel to instead rely on the ErrorReporter interface. Thanks to Kotlin's default arguments, we can still make Firebase our default tool - but ultimately the ViewModel no longer has any strict knowledge of Firebase itself.
class ProfileViewModel(
private val errorReporter: ErrorReporter = FirebaseErrorReporter()
) : ViewModel()
This gives us three big benefits:
- It is now easier to provide fake/mock implementations in our unit tests because we can implement the interface ourself.
- It becomes easier to entirely swap third party tools.
- It becomes easier to update our third party tools.
To provide an example for the second and third points, let's consider a global dependency injection manager that creates the ErrorReporter used in the application. If this is created in one single place, that means if we ever created a different ErrorReporter, we would only have to update that one line to update the whole project!
/**
* Change the reporter supplied by your DI manager,
* the entire app just updates!
*/
single<ErrorReporter> {
FirebaseErrorReporter()
// SentryErrorReporter()
// EmbraceErrorReporter()
}
Similarly, if for any reason the third party tool changes something about their method signature, we can handle that directly in our implementation class, without having to update the whole project:
class FirebaseErrorReporter : ErrorReporter {
/**
* Wrapping this method makes it easy to update our project
* if the library changes its method signature.
*/
override fun reportError(error: Throwable) {
// Firebase.getInstance().logException(error)
Firebase.getInstance().logError(error)
}
}
These Steps Avoid A Lot Of Legacy Code Pain Points
There's a lot more to discuss about wrapping dependencies and writing testable code, but the two steps we've discussed here (dependency injection and wrapping dependencies) will take us very far:
- We have testable code we can ship with confidence.
- We have decoupled third party tools, giving us the freedom to swap them with ease.
- We can confidently upgrade our third party tools and limit code changes required.
Now we'll look at one more cause of legacy code.
Legacy Code Doesn't Have Documentation
There is a lot that goes into writing good code comments, but code without any is likely to become legacy code.
This process happens because a developer may write code that they understand, and is understood by the code reviewer at the time. If there is some external knowledge used in writing that code though, it may not become clear to future teammates who have to inherit that code and they will end up unsure how it works, why it works, or if it can be changed.
Looking After Each Other With Documentation
When we write documentation, we don't write it for ourselves today. We write it for our future teammates who will see that code later. We should be thinking about them when we write our documentation to help them as much as possible.
Document When And Why You Rely On External Code Samples
If you include code that you've found on StackOverflow to solve a problem, it can be very helpful to document why you did that and where it came from. This helps solve a few problems:
- It lets your future teammates know why this code was added, and allows them to explore if new first party solutions exist.
- It can tell your future teammates where the source code came from, so they have a place to go with questions if they need to.
Here's an example of a helpful comment for this situation:
/**
* Feature XYZ requires a ViewPager like experience, but
* without the user being able to control each step. We've
* implemented a ViewPager that rejects any user interraction.
*
* Source: https://stackoverflow.com/a/9650884/3131147
*/
class NonSwipeableViewPager(
context: Context,
attrs: AttributeSet? = null
) : ViewPager(context, attrs) {
}
Document When You Work Around Library Bugs
Let's look at a very helpful example of writing a comment when you work around a bug in a third party tool:
/**
* Due to github.com/company/library/issues/1234 we needed
* to implement this work around to prevent a crash on
* Android devices running API 21.
*/
private fun doLibraryWorkAround() {
// ...
}
This comment helps your future teammates for a couple key reasons:
- It provides a link to the bug you are working around. This allows them to see if a new library update exists now that can replace this code.
- It gives a specific example of who was impacted by this work around, so they know what to test when they update the library.
Document Why You Use Third Party Libraries
Take this piece of advice with a grain of salt. Sometimes your project will rely on industry standard dependencies that we can expect people to understand. Other times, we may be using a tool for one specific scenario, and it can save our future teammates time by explaining why:
implementation("com.jakewharton:process-phoenix:$rootProject.ext.versions.phoenix") {
because("We use ProcessPhoenix to programmatically restart the application when network settings are changed.")
}
Why This Documentation Matters
This wasn't an exhaustive list of helpful documentation, but it can make a big difference. We're not trying to prevent code from becoming obsolete and outdated. We're trying to help the future maintainers respond accordingly.
Recap
- Writing testable code with decent coverage helps future developers refactor with confidence.
- Wrapping dependencies allows future developers to update and replace tooling easily.
- Documenting certain decisions allows future developers to confident understand, refactor, and replace code written by someone else.
Remember
The goal isn't to completely eradicate legacy code. The goal is to look out for our future teammates by not leaving behind rigid, confusing, and unmaintainable code.
I hope you learned some helpful tips and tricks from this post to avoid common legacy code problems. If you have any questions, or other ideas to avoid legacy code, let me know on Twitter or in the comments!
Latest comments (5)
OR Legacy code is just 10 year old code which needs to be supported as you don't have time/money to invest on rebuilding it... Sadly this is the case in a lot of businesses.
Yes of course. I was hoping to provide some ways we can make our 10 year old code a little more manageable. :)
Unfortunately we can't avoid 1 and 2. 4 is the usual reason for management to resist catching up on tech debt - to some extent I don't blame them because they don't see any explicit business value from resolving tech debt, while if doing so breaks anything there would be immediate business cost.
I definitely think "immediate" business cost is the difference. Tech debt has a long term business cost in that it can slow down and block certain types of development. It can get to a point where we can no longer work with it and we might feel we need to completely rewrite a feature to do what we want. This is a truth all engineers understand, but is hard to estimate and prove the long term impacts though.
Well, on top of hard to quantify the long term cost, it's even harder to convince people who only need to worry about the short-term impact, to care about the longer term - I mean, between the manager who is looking for the promotion after next successful release, and the contractor who knows their contract is up in another year...
Oh well, I feel like I'm borderline ranting at this point. :-)