Should I remove unreferenced code?

Question

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

Also clarify if these are public APIs that are tested in unit tests for customers. — Andrew T Finnell, Aug 23 '11 at 12:03

score 225 · Accepted Answer · answered Aug 23 '11 at 09:42

225

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.

answered Aug 23 '11 at 09:42

DeadMG

36,794
8
70
139

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 '11 at 09: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 '11 at 10:00
6

@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
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. – Andrew T Finnell Aug 23 '11 at 12:01
I wish I could upvote this to ∞. Very few things annoy me more than dead code. – CaffGeek Aug 23 '11 at 13:40
1

@Falcon - That counters the definition of "unused". – Rook Aug 23 '11 at 15:05
3

@Rook: But how can you tell that code is unused then in the first place by reading it in larger systems? Most IDEs will tell you that such code is unused. Some will remove it automatically, if you want. And then it'll break something. – Falcon Aug 23 '11 at 15:06
3

An addition to this answer, unused code also confuses new developers. A new developer will go in with the assumption things are working (and that all the code has some use). Dead code just causes them to chase breadcrumbs that lead the wrong direction. – jsternberg Aug 23 '11 at 15:29
@Falcon, then it was bad design that should be found. Or in that case there should have been a comment next to it saying this IS used, by where, so a human could verify it. – CaffGeek Aug 23 '11 at 15:30
@Chad: I agree, but under real working conditions, there're such issues and not everything is ideal. Real world software development is a mud covered beast somtimes. – Falcon Aug 23 '11 at 15:37
2

@Falcon...and that beast needs to be slain. Remove the code. Test. If it breaks because of reflection. Add the code back from source control. And determine if it's easy to code better, or if you need to leave a comment that "This seemingly dead code is called by reflection from foo" – CaffGeek Aug 23 '11 at 15:53
2

@Chad: I don't know why my comment caused such tidal waves. All I said is "be careful". – Falcon Aug 23 '11 at 16:00
1

@Falcon - That's why I don't remove code *just like that*. I first make it depreceated (replace it with something else), then obsolete (that something else didn't break anything else for a while), and if by then something isn't already broken, then delete it (replacement has worked for a longer period, and everything is still okey). – Rook Aug 23 '11 at 18:34
3

Removing it can be a good idea, but I really hate the argument that having it in version control is just as good as having it in the code--in fact nobody would know to go and look for it, so they would rewrite it from scratch. If I expected the code would really be used I'd leave "interesting code used to be here that did x" comments so that you'd know to go looking for it. – Bill K Aug 23 '11 at 18:52
@Bill K - Good point! – Rook Aug 23 '11 at 22:30
@Falcon Good automated tests will limit the damage refactoring can do. I find that refactoring code in a large project is near impossible without automatic test coverage. – Peter Graham Aug 24 '11 at 02:11
"If the code isn't used, throw it away- any benefit involved in keeping it can be trivially derived from source control". I find in practice that finding deleted code in source control is very difficult unless you have a good idea what you're looking for. Annotate/blame usually only shows insertions and changes, not deletions. I agree that unused code should generally be deleted though - usually if you need it back you can just rewrite it without much hassle anyway. – Peter Graham Aug 24 '11 at 02:15
+1 I really like the remove it, but leave a comment with information on which revision it can be found in. – Bjarke Freund-Hansen Aug 24 '11 at 06:53
1

Source control in this case is a red herring. If, and when, the functionality is needed it will be more cost effective to just write it from scratch again. – John Nilsson Nov 28 '12 at 00:38
Dead code is code someone invested time and effort (read money) in to cause you to waste time (read money) reading it and figuring out what it does, or waste time and complexity budget (read lots of money) on integrating, working around and align with new features. – John Nilsson Nov 28 '12 at 00:42
+1 If the code isn't used, throw it away- any benefit involved in keeping it can be trivially derived from source control. – 王奕然 Jul 23 '15 at 02:17
Worth to keep in mind - sometimes, not all code is visible to the IDE at once, and some references might go uncounted. Sure, maybe the solution "LibraryApp" counts zero references for "BankingValidationRoutine" in the "CommonLibrary.DLL", but that doesn't mean another solution - say, "FinancesApp" - doesn't use it. – T. Sar Apr 12 '21 at 16:55

score 45 · Answer 2 · answered Aug 23 '11 at 09:44

45

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.

answered Aug 23 '11 at 09:44

StuperUser

6,133
1
28
56

1

Yes, unreferenced code should not left in live code. – xdazz Aug 23 '11 at 16:04
Seems to me you also get these benefits by simply commenting big chunks out. – Steve Bennett Jan 04 '12 at 00: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 04 '12 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...) – Steve Bennett Jan 05 '12 at 07: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. – Zach Lysobey Nov 07 '14 at 16:06

score 24 · Answer 3 · answered Aug 23 '11 at 09:50

24

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.

answered Aug 23 '11 at 09:50

Nicholas Smith

1,621
10
11

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". – Scott Whitlock Aug 23 '11 at 15:01
Plus 1 for the much better improvement! – Nicholas Smith Aug 23 '11 at 15:52

Giorgio · Answer 4 · 2011-08-23T13:37:31.957

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.

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 08 '12 at 23:40

score 10 · Answer 5 · answered Aug 23 '11 at 14:47

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.

score 9 · Answer 6 · answered Aug 23 '11 at 14:49

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

score 3 · Answer 7 · answered Aug 23 '11 at 14:24

3

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.

answered Aug 23 '11 at 14:24

phresnel

554
2
12

score 3 · Answer 8 · answered Aug 26 '11 at 20:34

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

score 2 · Answer 9 · answered Aug 23 '11 at 18:07

2

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.

answered Aug 23 '11 at 18:07

davorp

131
3

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 '13 at 14:08

score 1 · Answer 10 · answered Aug 23 '11 at 13:24

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.

score 1 · Answer 11 · answered Aug 23 '11 at 22:24

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.

score 1 · Answer 12 · answered Aug 23 '11 at 22:39

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.

score 1 · Answer 13 · answered Aug 23 '11 at 23:43

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

score 1 · Answer 14 · answered Aug 24 '11 at 02:29

Follow this simple algorithm:

Is it backed up in an SCM? If yes, jump to 3.
Set up an SCM.
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.

score 0 · Answer 15 · answered Aug 23 '11 at 15:55

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

score 0 · Answer 16 · answered Aug 23 '11 at 22:12

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.

Should I remove unreferenced code?

16 Answers16

Linked