Deleting code
Created 22 December 2002, last updated 6 November 2012
This document is also available in Russian.
There’s plenty of information out there about how to write code. Here’s some advice on how to delete code.
The best way to delete code
This may seem obvious, but I guess it isn’t, because of the variety of other ways developers have of deleting code. Here’s how to delete code:
Select a section of code in your editor, hit the backspace key, and be done with it.
Most developers don’t like getting rid of stuff. They want to keep chunks of code around in case they need them again. They worked hard to write that chunk of code. They debugged it, it works. They don’t want to just throw it away.
These developers want to keep their old code around, and they do it by disabling it in some way: commenting it out, conditionalizing it, or just not calling it anymore.
To those developers, I say, “Use the source (control), Luke”. A source code control system (like Git, Mercurial, or Subversion), means you never have to worry that something is gone forever. Your repository will have the old code if you need it again.
If you don’t have a source control system (!?!?!) or just don’t want to be bothered digging back through the history, then copy the chunk of code to a separate file some place, and save it away. But don’t leave it where it doesn’t belong: in your source code.
What’s the big deal?
If you have a chunk of code you don’t need any more, there’s one big reason to delete it for real rather than leaving it in a disabled state: to reduce noise and uncertainty. Some of the worst enemies a developer has are noise or uncertainty in their code, because they prevent working with it effectively in the future.
A chunk of code in a disabled state just causes uncertainty. It puts questions in other developers’ minds:
- Why did the code used to be this way?
- Why is this new way better?
- Are we going to switch back to the old way?
- How will we decide?
If the answer to one of these questions is important for people to know, then write a comment spelling it out. Don’t leave your co-workers guessing.
Commenting out code
It’s very easy to comment out a line or two (or twenty!) lines of code:
// OldWayStepOne(fooey);
// OldWayStepTwo(gooey);
NewWay(fooey, gooey);
This is bad. Comments should be used to provide people with information they need when reading or writing the code. They should be used to help the future developers who will be working with the code. These comments don’t do that. In fact, they do just the opposite. In addition to removing the old code from being compiled, these comments add confusion, uncertainty, and doubt into the code.
Future developers looking at this code know that it used to work the OldWay, and they know that now it works the NewWay, but they don’t know why the OldWay has been kept around:
- Maybe NewWay is just an experiment? If so, what’s better about it? How and when will the final decision be made to keep it?
- Maybe OldWay is better, but there was something wrong with it? If so, what was wrong with it? Was it something wrong with the OldWay code, or they way we’re calling it? When will it be fixed?
- Maybe the design has changed, and OldWay is doing unnecessary work?
Any commented-out code is an implicit question: Why is this still here? There are reasons to keep a piece of commented-out code. Changes get made that you know will be reversed soon. Changes get made that the developer is uncertain of. It’s OK to keep the code, but say why you’re keeping it. Comments are for people, and a line of code in a comment doesn’t tell anyone anything.
Don’t comment out a piece of code without saying why (in the comment).
Isn’t this better?:
// OldWay did a better job, but is too inefficient until MumbleFrabbitz
// is overhauled, so we'll use NewWay until the M4 milestone.
// OldWayStepOne(fooey);
// OldWayStepTwo(gooey);
NewWay(fooey, gooey);
Now, who knows if MumbleFrabbitz will really be overhauled for the M4 milestone? Maybe it won’t be. That’s OK; who knows what the future will bring? But at least this way the developers will know why the code is being kept around. With the information about why the change was made, and why the old code is still there, the developers will know when they can finally fully commit to the NewWay, or when they can switch back to the better solution.
Conditional compilation
Developers who want to comment out large chunks will use conditional compilation instead (if the language supports it). In C++:
#if 0
OldWayStepOne(fooey);
...
OldWayStepTwenty(hooey);
#endif
In Python:
if 0:
OldWayStepOne(fooey)
...
OldWayStepTwenty(hooey)
This is no better than commenting out the code: it’s just more convenient for whoever is doing the removing. In fact, in some ways it is worse than commenting out the code. Some IDEs don’t syntax-color this code as a comment, so it’s easy for other developers to read this code and not realize it has been disabled.
The same rule applies as for commenting out code:
Don’t conditionalize away code without explaining why.
If you must use the C preprocessor to remove code, “#if 0” is really the best way to do it, since it is at least clear that the code should never be compiled.
At Lotus, the source code for Notes include many lines of code removed with “#ifdef LATER”, under the (correct) assumption that there was no preprocessor symbol called “LATER”. This is a very weak form of documentation; it indicates that the code isn’t ready to be compiled yet, but that it will be later. But when? A running joke among the developers was that we should define “LATER” and see what happened!
By using never-defined symbols to remove code, you leave doubt in developers minds as to what the symbols mean. Maybe there’s a configuration of the code called “LATER” that has to be taken into account.
Uncalled code
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. 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. (I’ll have to write another piece about framework vs. application).
Leaving pointers
One compromise that you might consider is to remove a large chunk of unused code, but leave behind a pointer to where it could be found if it were needed. I’ve used comments like this before:
// (There used to be another algorithm here that used hashing, that
// was faster, but had race conditions. If you want it, it's in
// commit 771de15b or earlier of ThingMap.java in the repo.)
It’s small, it’s unobtrusive, but it gives a little history, and a place to go looking for more information.
Accidental droppings
Sometimes, while writing code, you really are unsure about whether to keep or delete a line of code, and you want to try compiling or running the code before you decide what to do. You comment out the line. A number of files get changed, and by the time you are ready to check in the code, you’ve forgotten where all those temporary removals are. You check in the code, and you’ve left accidental droppings all over the place.
Always use a distinctive marker in your commented-out lines of code, so you can quickly find them all when it’s time to clean up and check in.
A simple convention like this:
//- OldWayImUnsureOf(zooey);
makes all the difference. By using “//-” to comment out the line, you’ve left a marker that you can easily find when you are getting ready to check in your code.
You can use it for larger chunks as well:
#if 0 //- I don't think I need this with the new FooBar
OldWayStepOne(fooey);
...
OldWayStepTwenty(hooey);
#endif
Keep things tidy
While deleting code, it is all too easy to leave phantom stubs behind. Try hard to trim these properly. For example, when getting rid of OldWay here:
if (bDoThing) {
OldWay();
}
Don’t just take out the line calling OldWay. Get rid of the empty if as well. Then if bDoThing was only tested here, also get rid of it. Examine the code that set bDoThing. Is it now obsolete? Get rid of it. Be merciless. Keep the code tidy. Make sure it makes sense with no dead-end off ramps that can only be understood by knowing what used to be there.
It is tempting to leave this code in, because it will be difficult to understand whether it is all still needed or not. But if you leave the empty if clause, some other developer will come along later, and see it, realize it can’t be right, and have to investigate. It will take them longer to understand the empty if than it would take you to remove it.
Don’t worry, be happy
I know it seems drastic to just chop out code that you sweated over. Don’t worry: it will be OK. There’s a reason you wanted to disable it or whatever. The source control system will still have a copy if you need to go back to it. Look at it this way: what are the chances you need to go back and get it, compared to the certainty that you’ll have to be looking at those stupid commented-out lines for the rest of the project’s life?
Go ahead, delete that old code. You won’t miss it.
See also
- Erroneously empty code paths, about mistakes in defensive coding.
- Fix error handling first, about ensuring that your error handling code is always running its best.
- My blog, where other similar topics are discussed.
Comments
Be happy in the best way forever.
You tell them.
IMO, using diff command before check-in is a better approach. It'd detect not only commented out lines, but accidentally deleted ones as well.
is commented out, and then the real code changes, the old code is not only obsolete, but it may cause hard -to-detect errors,
as for the special marks :
i use /// in javascript or ## in tcl
and of, course, vim: /\/\/\/.*/
Can anyone recommend a tool which will detect commented Java code? I have a large code base that would be too big to check by eye.
I've been experimenting with PMD (http://pmd.sourceforge.net/) but there doesn't seem to be a rule for commented code. I'm considering writing one, but before I do, just checking no-one has done this already...
This will help for fast coding..
Relying on version control doesn't make much sense to me--I've seen too many VC systems screw up, leaving people high-n-dry. Redundancy, redundancy, and more redundancy. If the swath of code is really large, I move it to a folder called dino, short for dinosaur.
Never rely on VC or nightly tape backups; they are just conveniences. tar or zip and a CD burner are your friend. So is RAID. Of course you don't have to burn a CD every night, but what if the tapes turn out to be duds? Then what are you gonna do? Did I mention redundancy. OK, this strays off the topic a bit; the real point is that history matters.
Sometimes, knowing what you threw away is important. When it's time for maintenance programmers to come in and do things, then it's probably appropriate to "clean house", but otherwise I'll continue to do what I've always been doing. It seems to be working quite well, and if it ain't broke I'm not going to fix it. Did I mention redundancy?
label the tag with some detail and voila.
You *could* argue that this might not work with other environments however in my experience most people standardise on vs.net in the .net dev world...
First you do
// #define USE_OLD_WAY 1
#if USE_OLD_WAY
// BUGBUG - remove this after testing
// old code
#else
// new code
#endif
and then after it's made it through a few test passes, it's safe to yank.
0x80240016.
in some .h file:
#define ENABLE_STUFF
in a .c/.cpp file
.
.
#ifdef ENABLE_STUFF
//use stuff
#endif
But just before a checkin, or shortly after, I delete the unreachable code.
Source Safe keeps always a copy that I could return to.
The following example is NOT a way to disable code, rather a way to get some output when debugging an application:
#if(DEBUG)
MakeSomeDebugPrint();
#endif
grep -F '//' *.cpp
Erm... Sentimental value is not a good enough reason? *rolling eyes*
LOL! Nice write up! :)
I was picking apart a complex C# class today and noticed a partial class that never got used. Right to the bit bucket...
I'm not sure if you mention it, but using version control is good for getting the psychological freedom to delete code. It always helps to be able to tell your co-workers (or yourself) that you can get it back in a few minutes when you need it.
Even here what I am writing is in code but gets received in English.Anytime I fill in a comment space or message on a card it goes in code. How do I get ridof it?
Discoverability:
How do I or anyone else know the code is there? Do I have to look at every revision of every class I touch? Surely, the fact that a particular revision may be of use to later development is something that should be explicitly communicated?
No-Commit:
What If I write some code, that may be useful, but isn't needed not (like a WIP, or another way to do something). The effort in creating that code may make it worthwhile to keep around, but I haven't actually committed it - In this case I might have to create *at least* two commits - one to put the code in revision, and a second to remove it...
Discoverability: If you need to know revert to a prior version of the code, then you would already be looking at the repo history. You only need to look at the revisions when you need to revert to a previous state or (maybe) need to do some research on why things changed.
No-Commit: If you've written code that isn't currently useful, then you've wasted time. Why would you write something you don't currently need? If you still wanted to write the code, you could commit that to a separate branch and wait for it to be merged into the master branch.
If commented-out code or web-ui is not used for some time, it is trivial to delete it when you absolutely know you don't need it anymore.
In the "Leaving Pointers" section, how do you get the commit hash before you have made the commit? Only way I can think of is to make the removal, then get the hash and make a second commit.
Hopefully you have another trick. :)
The difference for me is that I always INSIST in my teams that commented code be removed. Never leave commented out code in the source. It is OK for a couple of days, but no more than that. It is basically trash -- a kind of technical debt that will burn you eventually. Better to clean it out. Here is a link to a write up on this.
https://agiletribe.wordpress.com/2015/12/26/never-leave-commented-code-in-the-source/
Add a comment: