A few days ago, Ned Batchelder's post on deleting code made the rounds on HN, even though it was originally written in 2002. Here I want to echo a few of Ned's points, and take a stronger stance than he did: delete code as soon as you know you don't need it any more, no questions asked. I'll also offer some tips from the trenches for how to identify candidate dead code.
This is the second in an ongoing series on eating your vegetables in software engineering, on good, healthy practices for a happy and successful codebase. I don't (yet) know how long the series will be, so please stay tuned!
What Is Dead May Never Die
This heading isn't just an oh-so-clever and timely pop culture reference. Dead code, that is, code that can't possibly be executed by your program, is a real hindrance to the maintainability of your codebase. How many times have you gone to add what seemed like a simple feature or improvement, only to be stymied by the complexity of the code you have to work around and within? How much nicer would your life be if the practice of adding a feature or fixing a bug was as easy as you actually thought it would be during sprint planning?
Each time you want to make a change, you must consider how it interacts with each of the existing features, quirks, known bugs, and limitations of all the code that surrounds it. By having less code surrounding the feature you want to add, there's less to consider and less that can go wrong. Dead code is especially pernicious, because it looks like you need to consider interactions with it, but, since it's dead, it's merely a distraction. It can't possibly benefit you since it's never called.
The fact that dead code might never actually die is an existential threat to your ability to work with a given codebase. In the limit, if code that isn't called is never culled, the size of your application will grow forever. Before you know it, what might only be a few thousand lines of actual functionality is surrounded by orders of magnitude more code that, by definition, does nothing of value.
It's Got to Go
Ned (Batchelder, not Stark) was a little more nuanced and diplomatic than I'm going to be here:
Let's say you have a great class, and it has many methods. One day you discover that you no longer are calling a particular method. Do you leave it in or take it out?
There's no single answer to the question, because it depends on the class and the method. The answer depends on whether you think the method might be called again in the future.
I say: scorch the earth and leave no code alive. The best code is code that you don't even have.
For those less audacious than I, remember that version control has your back in case you ever need that code again.
That said, I've never experienced a need to add something back that I have previously deleted, at least not in the literal sense of adding back in, line for line, verbatim, a section of code I'd previously deleted.
Of course I'm not talking about things like reverting wrong-headed commits here -- we're all human, and I make as many mistakes as the next person. What I mean is, I've never deleted a feature, shipped to production, then come back weeks, or months later and thought to myself, "boy howdy, that code I wrote a year or more ago was probably pretty good, so let's put it back now." Codebases live and evolve with time, so the old code probably doesn't fit with the new ideas, techniques, frameworks, and styles in use today. I might refer back to the old version for a refresher, especially if it's particularly subtle, but I've never brought code back in, wholesale.
So, do yourself -- and your team -- a favor, and delete dead code as soon as you notice it.
How Did We Get Here?
Ned's post goes into great detail on how and why dead code happens -- perhaps the person making a change didn't think the code would be gone forever, and so commented it out or conditionally compiled it. Perhaps the person making a change didn't know enough to know that the code was actually dead (about which more later).
I'll add another hypothesis to the list: we might all just be lazy. It's definitely easier not to do something (i.e. to leave the code as-is) than to do something (delete it).
Laziness is, after all, one of the three great virtues of a programmer. But the Laziness that Larry Wall was talking about isn't this kind, but another kind: "The quality that makes you go to great effort to reduce overall energy expenditure." Viewed this way, deleting dead code is an act of capital-L Laziness -- doing something that's easy now to prevent yourself from having to do something hard later. We could all stand to develop more of this kind of Laziness, what I like to think of as "disciplined laziness," in our day-to-day habits.
How Do We Get Out Of Here?
I spend most of my time programming in Python, where, unfortunately, IDEs can't usually correctly analyze a complete codebase and identify never-called code automatically. But, with a combination of discipline and some run-time tooling, we can attack this problem from two sides.
For simple cases, a better sense of situational awareness can help identify
and remove dead code while you're making changes. Imagine you're working on
a particular function, and you notice that a branch of an
can't be executed based on the valid values of the surrounding code. I call
this "dead code in the small," and this is quite easy to reason about and
remove, but it does require a bit more effort than one might ordinarily
Until you develop the habit of noticing this during the course of your ordinary programming routine, you can add a step to your pre-commit checklist: review the code around your changes for any now-dead code. This could happen just before you submit the code to your co-workers for review (you do do code review, right?) so that they don't have to repeat that process while reading through your changes.
Another kind of dead code happens when you remove the last usage of a class or function from within the code you're changing, without realizing that it's the last place that uses it. This is "dead code in the large," and is harder to discover in the course of ordinary programming unless you're lucky enough to have eidetic memory or know the codebase like the back of your hand.
This is where run-time tooling can help us out. At Magnetic, we're using Ned's (yes, the same Ned) coverage.py package to help inform our decisions about dead code. Ordinarily coverage is used during testing to ensure that your test cases appropriately exercise the code under test, but we also use it within our code "running as normal" to get insight into what is or isn't used:
import coverage cov = coverage.Coverage( data_file="/path/to/my_program.coverage", auto_data=True, cover_pylib=False, branch=True, source=["/path/to/my/program"], ) cov.start() # ... do some stuff ... cov.stop() cov.save()
This sets up a
Coverage object with a few options that make the report
more usable. First, we tell coverage where to save its data (we'll use that
later to produce a nice HTML report of what is and isn't used), and ask it
to automatically load and append to that file with
auto_data=True. Next we
ask it not to bother calculating coverage over the standard library or in
installed packages -- that's not our code, so we'd expect that a lot of
what's in there might not be used by us. It's not dead code that we need to
maintain, so we can safely ignore it. We ask it to compute branch coverage
(whether the true and false conditions of each
if statement are hit). And
finally, we point it out our sources, so that it can link its knowledge of
what is or isn't called back to the source code for report computation.
After our program runs, we can compute the HTML coverage report like:
$ COVERAGE_FILE=/path/to/my_program.coverage coverage html -d /path/to/output
Which generates a report like:
The lines highlighted in red are lines that were never hit during the recorded execution of the program -- these are candidate lines (and, by extension, methods) for dead code elimination.
I'll leave you with three warnings about using this approach to finding and removing dead code:
- Be careful when reviewing the results of a coverage run -- the fact that a line or function wasn't executed during a single run of the program doesn't mean they're necessarily dead or unreachable in general. You must still check the codebase to determine whether they're completely dead in your application.
- Computing coverage means your program needs to do more work, so it will become slower when run in this mode. I wouldn't recommend running this all the time in production, but in a staging environment or in targeted scenarios you'll probably be OK. As always, if performance is an important concern, you should definitely measure what impact coverage has before you run it.
- Finally, dont trust code coverage reports from testing runs to find dead code. Some code might be dead, save for the tests that excercise it; and some code might be alive, but untested!
To you, dear reader, I must apologize. I left out an important part of Ned's blog post when I quoted him earlier. He says:
There's no single answer to the question, because it depends on the class and the method. [...] A coarse answer could be: if the class is part of the framework, then leave it, if it is part of the application, then remove it.
If you're an author of a library or framework, rather than an application, then the question of dead code becomes in some ways harder and in other ways easier. In essence, you can't ever remove a part of your public API (except during major version bumps). Essentially, all of your public API is live code, even if you, yourself, don't use it. But behind the public interface, dead code can still happen, and it should still be removed.
Delete your dead code!