125
votes

I'm working on a medium sized (100k lines) code base, it's all relatively recent code (less than a year old) and has good unit test coverage.

I keep coming across methods which are either no longer used anywhere or only referenced in unit tests which only test that specific method.

Should I remove this code if I'm certain that it's no longer needed?

Reasons to remove it:

  • Less code, less bugs
  • Less code is easier for others to digest
  • It's still under source control

Reasons to keep it:

  • Can be used as reference
  • It may be useful sometime
  • It may have been written to 'round-out' the functionality for a class
1
  • 2
    Also clarify if these are public APIs that are tested in unit tests for customers. Aug 23, 2011 at 12:03

16 Answers 16

225
votes

Most of your reasons to keep it are utterly irrelevant, put simply. If the code isn't used, throw it away- any benefit involved in keeping it can be trivially derived from source control. At most, leave a comment saying which revision to find it in.

Quite simply, the sooner you cut the code, the sooner you don't have to waste time maintaining it, compiling it, and testing it. Those advantages massively outweigh the trivial benefits that you have outlined, all of which can be derived from source control anyway.

23
  • 31
    I agree with removing it, but I've had cases where I broke something because someone was using the "unused" code via reflection. So one should be careful anyway.
    – Falcon
    Aug 23, 2011 at 9:52
  • 14
    @Falcon, that's a good reason to remove it as soon as possible, before people start using it, or discover the need to make it public.
    – StuperUser
    Aug 23, 2011 at 10:00
  • 6
    @Falcon: It's the OP's assertion that the code is unused.
    – DeadMG
    Aug 23, 2011 at 10:02
  • 4
    @StuperUser: I totally agree. But one should be careful and prepare for the unexpected.
    – Falcon
    Aug 23, 2011 at 10:09
  • 18
    If you remove it and your unit and regression tests pass but the product breaks in the field it provides a strong case for setting some type of code coverage tools. Aug 23, 2011 at 12:01
45
votes

All of the reasons to remove it stand.

Reasons to keep it:

  • Can be used as reference
  • It may be useful sometime
  • It may have been written to 'round-out' the functionality for a class

All of these reasons to keep it will be managed by source control. Remove it from the live code and you will be able to retrieve it if/when it's needed.

5
  • 1
    Yes, unreferenced code should not left in live code.
    – xdazz
    Aug 23, 2011 at 16:04
  • Seems to me you also get these benefits by simply commenting big chunks out. Jan 4, 2012 at 0:56
  • @SteveBennett Indeed, but you misunderstand the point of this answer. I list the benefits of keeping it commented, the point is that you will get all of these AND MORE benefits from source control (which you will see in other answers).
    – StuperUser
    Jan 4, 2012 at 10:03
  • I'm certainly not against VCS. :) (I do note though, that once stuff is removed from the current version, it's much less visible than other approaches, like storing it in an unreferenced text file, on a wiki, in comments, etc...) Jan 5, 2012 at 7:49
  • @Steve Bennet - It may be "less visible" than comments in the current version of a file, but its trivial to check the VCS history of a single file, and I would say significantly easier than the txt-file/wiki/etc... approaches. Nov 7, 2014 at 16:06
24
votes

Unreferenced code is the same as keeping those batteries that are kinda, sorta flat just in case you need them one day for a torch.

As long as you're using some kind of version control I'd say trash it out of the live code and use the versioning history in case it turns out to be useful.

2
  • 44
    To improve slightly on your analogy, it's kind like keeping the almost dead batteries taped to your remote control, instead of in a box next to the new batteries in the storage room labelled "used but not dead batteries". Aug 23, 2011 at 15:01
  • Plus 1 for the much better improvement! Aug 23, 2011 at 15:52
14
votes

The only good reason I can see to keep code that isn't currently used is if it is part of a self-contained module: While some parts of the code might not be used at the moment, it may well be that they will be used in the future.

This may be especially true for a library that you use across different projects: you do not want to keep throwing pieces of code in and out, according to what you need for a specific project: I find this time consuming and error-prone.

My approach is: (1) if you use it once, only keep what you really need; (2) if you use it twice, copy and adapt it the second time you use it; (3) if you use it more than twice, make a well-defined, stable module out of it, and use this module as you need.

Summarizing: I would throw away all unused code unless it is part of a general-purpose module that you have designed as such and that you know you are going to reuse several times.

Note: Of course, an even cleaner solution would be to make a separate project for a library, and add a dependency between projects.

1
  • 1
    As an example of this, I have some library routines which to read and write various-sized signed and unsigned data types within a byte array. It seems cleaner to have a set of such routines for all data types, than to have some be present and some not, based upon what the code happens to require at the moment.
    – supercat
    Oct 8, 2012 at 23:40
10
votes

Generally, I would bow to YAGNI on this. If "you ain't gonna need it", then it is simply taking up space in your codebase, unit tests, and assemblies. You may end up needing it, but you may also end up needing to completely rewrite it because between now and when you need anything like it, a lot can change.

However, that changes somewhat when you're writing a utility or API meant for general consumption. Just like you can never expect software end-users to interact with the software in the way you intended, you can never expect consumers of your code to want to use your code exactly the way you think they should. In such cases, as long as you can justify a method's existence with "it's a valid way to want to interact with my object" then it should probably go in, because even if you ain't gonna need it, chances are good SOMEONE will.

9
votes

Given that the codebase is less than a year old, it's probably still in a lot of flux (yes?) - so the notion that some bits might need to be resurrected in the near future is not unreasonable.

For bits that were hard to get right in the first place and seem more likely to be resurrected, I'd keep them a bit more "live" than just in source control. Folks won't know/ remember that they exist - saying "you can just find it in source control" presumes that you know/remember that it's there! In these kinds of cases, consider deprecation (with an "assert(false)" showstopper) or commenting out.

1
  • 5
    +1 for "saying 'you can just find it in source control' presumes that you know/remember that it's there!" - sometimes I find little reminders of code that was cut to be helpful, for some reason or other.
    – Steven
    Aug 23, 2011 at 15:10
3
votes

If the code is trivial and uninteresting, I just throw it away to rid of unnecessary inertia of the software system.

For interesting code cadavers, I use an archive branch in my version control systems.

3
votes

"Can be used as a reference" I would not tend to agree with being a good reason to leave in unused code. Often, only a small portion of the unused code is actually demonstrating something interesting. There are multiple ways to document and store useful but unused code.

Although version control will contain a history which will easily allow you to restore particular functionality if you later decide the code is needed, knowing that you need to go look in the version control history to find x y or z from who knows what previous revision can be a bit tedious, and often is overlooked unless you have a fairly specific idea what you are looking for.

The code could be commented out with a note about when it was removed and why it wasn't simply deleted from the code. However, this is generally considered bad style, and code that isn't used and not maintained properly can introduce all sorts of bugs if it is uncommented later on, so this is generally better as a temporary debugging/testing step while mid-refactoring than a way to leave production code.

My favorite way to store the deleted code, if it appears to be useful in the future, is to make a secondary reference document containing all the various chunks of worthwhile deleted code. Each block of code is labeled with a brief mention of where it came from or anything else pertinent to remember such as when it was removed or the revision number it was last in the code at. Everything removed but "potentially useful" is in one place, easily searchable, but not requiring constant effort to maintain and test on an ongoing basis (that testing is defered to whatever point the code is re-introduced).

2
votes

One good reason to keep the unused methods is: they could be used on other branches/tags!

Explore all your active branches and tags before removing them.

1
  • 1
    That does not make sense to me: If it's not used in this branch, remove it from this branch. If another branch uses it, it will stay there.
    – sleske
    Sep 26, 2013 at 14:08
1
vote

If you use a version control system, then don't worry about future references, as you simply can see through the history of that code and find the deleted part. If you don't and you think it would be used someday, then simply let it stay there, but comment it with a description explaining why it's commented.

However, if you're sure that you won't use it in future, simply remove it. I think the reasons you've mentioned are pretty straightforward for the code to be removed.

But please before removing it, make sure that it's not used anywhere. Visual Studio has a feature called Find All References which searches the entire solution and finds any reference to the current variable, method, property, class, interface, etc. I always get sure that no reference is in place before removing part of my code.

1
vote

I've relatively frequently had the experience of finding a function that looks like it will do exactly what I need, and trusting it to work since it's been in production for a long time, only to find out it hasn't actually been used in several years. Code that isn't used isn't maintained, and while it may have worked years ago, the API all around it has changed enough that you can't trust it.

At best, you end up spending a lot of time making sure it actually still does what you want. At worst, it appears to work until you get bitten with a nasty bug later, and take longer tracking it down because you assumed the "production-tested" code works so the problem must be somewhere else in your new code. In my experience, it's almost always quicker just to write a new function myself.

If you delete it, but find out relatively soon you actually need it, it's right there in source control. If you don't need it until so much time has passed that you don't remember it's in source control, you're probably better off writing it from scratch anyway.

1
vote

Nothing consumes less time than no code.

If you have to dive into a codebase, you need some time to figure out, what that code is used for, and if it is used for nothing, you need even more time.

Ok - this could be healed be a comment, but then again, everybody will reason about why this unused code is still in the code base, whether it should be removed or not.

If there is nothing, nobody will lose time with it.

If it was hard to get it right, you need a good documentation, that this code exists, but if the codebase evolves over some iterations, maybe it will not longer work, if it gets reactivated.

1
vote

Remove unused code - less clutter, better understanding. Your version control system will take care. Also, if you are using anything better than notepad, your environment will allow you to use older code for reference.

Long comments of old code are distracting and make difficult navigating.

Regards

1
vote

Follow this simple algorithm:

  1. Is it backed up in an SCM? If yes, jump to 3.
  2. Set up an SCM.
  3. Throw the stuff away.

All your points in favor of removal are valid.

All your points in favor of keeping the clutter are invalid once you have an SCM to look it up or restore it. Actually, your SCM should be able to help you determine why that code is here and unused, if it was used properly.

It's called "dead code" for a reason. Let it die and rest in peace.

0
votes

It will stay in source control; it should not stay in the active code base.

The one exception is if the code completes the design and while you aren't using the code, you plan on eventually making the code public and others may want that basic functionality. Then just design tests to make sure these parts of the code work that mimic how you propose others may use these parts of the code. However, if you are even considering deleting the code and can't come up with a solid reason to keep it, it should go. Unused code makes more work for everyone (harder to read the code; the code may be broken; more work to maintain; etc.)

0
votes

In my experience, removing unused code can backfire too. You may forget you had that code and you won't look for it in the history. Or you may not even know someone had implemented that code and then removed it later. Again, you won't look for it in the history...

No doubt, having unused code is a bad smell.

UPDATE: I just noticed Ed Staub gave a very similar answer.

Not the answer you're looking for? Browse other questions tagged or ask your own question.