Creating A Better Developer Experience By Avoiding Legacy Code

Adam McNeilly - Oct 9 '20 - - Dev Community

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.

  1. Code that was written before we knew better - new tools and practices exist now.
  2. Code that was written by someone else who left our team - taking institutional knowledge with them when they left.
  3. Code that doesn't have any unit test coverage.
  4. Code that we're unable to/afraid to change because we don't know what might break.
  5. Code without any type of documentation or comments.
  6. 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:

There are ultimately two reasons that code doesn't have tests.

  1. The developer chose not to write any - this can happen for a number of reasons.
  2. 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)
        }
    }
}
Enter fullscreen mode Exit fullscreen mode

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:

  1. 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.
  2. We don't want our negative tests to actually record errors to our production Firebase tool. That would just create noise in our dashboard.
  3. 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)
        }
    }
}
Enter fullscreen mode Exit fullscreen mode

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()
Enter fullscreen mode Exit fullscreen mode

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)
}
Enter fullscreen mode Exit fullscreen mode

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)
    }
}
Enter fullscreen mode Exit fullscreen mode

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()
Enter fullscreen mode Exit fullscreen mode

This gives us three big benefits:

  1. It is now easier to provide fake/mock implementations in our unit tests because we can implement the interface ourself.
  2. It becomes easier to entirely swap third party tools.
  3. 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()
}
Enter fullscreen mode Exit fullscreen mode

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)
    }
}
Enter fullscreen mode Exit fullscreen mode

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) {
}
Enter fullscreen mode Exit fullscreen mode

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() {
    // ...
}
Enter fullscreen mode Exit fullscreen mode

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.")
}
Enter fullscreen mode Exit fullscreen mode

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!

. . . . . . . . . . . . . . . . . . . . . . . .