Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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
share|improve this question
18  
"Less code, less bugs" - if they are truly never used, they are unlikely to cause bugs –  Konrad Morawski Aug 23 '11 at 10:36
14  
@Morawski But if it is left in, it will one day be used. And since it hasn't been maintained it will have bugs in, which will then appear. –  DJClayworth Aug 23 '11 at 14:16
7  
I usually would comment it and the tests that depend on it out, and then leave a 'TODO' comment with a date. Once it has not been used for a year, I chuck it. Others recommend just removing it now, but I find that hard to do, particularly if it looks like the code was once useful. –  Job Aug 23 '11 at 14:31
22  
@Job If I come across commented out code it is removed. No excuses. Commented out code just screams "I don't trust our source control system." to me. –  Kristof Provost Aug 23 '11 at 15:58
21  
@Kristof Provost, how would you know that useful code was once in source file A there if it is no longer there? of course, you can always check the history of the file you are on, but how often do you think to yourself: "Hm ... I need to change/implement a feature here. Or I need to test how this worked 5 years ago QUICKLY. I wonder if anyone has implemented it already and then deleted it ... let me check the history". I am not advocating that messy crap is kept around, but there are circumstances when you are not using the code in production, but do/may need it on occasion for debugging, etc. –  Job Aug 23 '11 at 16:59

16 Answers 16

up vote 196 down vote accepted

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.

share|improve this answer
23  
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 '11 at 9:52
11  
@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 '11 at 10:00
5  
@Falcon: It's the OP's assertion that the code is unused. –  DeadMG Aug 23 '11 at 10:02
4  
@StuperUser: I totally agree. But one should be careful and prepare for the unexpected. –  Falcon Aug 23 '11 at 10:09
12  
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. –  Andrew Finnell Aug 23 '11 at 12:01

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.

share|improve this answer
1  
Yes, unreferenced code should not left in live code. –  xdazz Aug 23 '11 at 16:04

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.

share|improve this answer
32  
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". –  Scott Whitlock Aug 23 '11 at 15:01

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.

share|improve this answer

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.

share|improve this answer

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.

share|improve this answer
3  
+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 '11 at 15:10

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.

share|improve this answer

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.

share|improve this answer

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.

share|improve this answer

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.

share|improve this answer

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

share|improve this answer

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.

share|improve this answer

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.)

share|improve this answer

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.

share|improve this answer

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.

share|improve this answer

"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).

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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