124

You've found some code that looks superfluous, and the compiler doesn't notice that. What do you do to be sure (or as close to sure as you can) that deleting this code won't cause regression.

Two ideas spring to mind.

  1. "Simply" use deduction based on whether or not the code looks like it should execute. However, sometimes this can be a complex, time-consuming job, slightly risky (your assessment is prone to error) for no substantial business return.

  2. Place logging in that code section and see how often it gets entered in practice. After enough executions, you should have reasonable confidence removing the code is safe.

Are there any better ideas or something like a canonical approach?

Peter Mortensen
  • 1,050
  • 2
  • 12
  • 14
Bradley Thomas
  • 5,090
  • 6
  • 17
  • 26
  • Comments are not for extended discussion; this conversation has been [moved to chat](http://chat.stackexchange.com/rooms/55018/discussion-on-question-by-brad-thomas-how-do-you-safely-delete-a-piece-of-code-t). – maple_shaft Mar 08 '17 at 17:33
  • 56
    It may be helpful to look at version control history: When was the "dead code" created? Did it look dead when it was created? Was other code (which has since been modified or deleted) checked in which used it, at the same time as it was created? – ChrisW Mar 08 '17 at 15:59
  • 9
    The compiler can't be trusted if any form of meta-programming such as reflection is in play. A proper grep of the base directory is an essential practice. It is also common to have multiple applications sharing code so be sure to grep at the correct directory layer. – Adam Caviness Mar 08 '17 at 21:41
  • 16
    I would ask - "Why bother to delete it?". If it's used in some edge case that is called once in a blue moon, you've broken things. If it's never used and you leave it there, the damage is that the executable file is a tiny bit larger. Unless this is for some embedded device, this is a big deal? Stick a comment on it and move on to using your time productively. – MickeyfAgain_BeforeExitOfSO Mar 08 '17 at 21:51
  • 1
    delete it, ship it, wait for a bug report, fix it, ship it, repeat.. – user428517 Mar 09 '17 at 00:03
  • 3
    @sgroves I would highly discourage from doing that provided you care about bugs in the software that you write. You can't be certain to ever get a bug report for that bug, even if removing that code actually caused a bug. – HelloGoodbye Mar 09 '17 at 02:19
  • 41
    @mickeyf The damage is that someone has to maintain that code. That means a developer has to understand what it does. The developer has to modify it if things change and the code around it now has to do something different. Believe me, when you leave cruft lying around, it's a major headache for anyone trying to figure things out later. – jpmc26 Mar 09 '17 at 02:24
  • 2
    @jpmc26 So the OP either understands that it *does nothing*, and deletes it. Or he *does not understand it* entirely, which is the situation described here - he has doubts. And yet you're telling him to delete it? – MickeyfAgain_BeforeExitOfSO Mar 09 '17 at 03:38
  • 4
    @mickeyf No, I'm explaining the value of deleting it because you asked about it. It's therefore worth at least some effort to look into whether it can be deleted. Don't put words in people's mouths when they answer questions *you* posed. – jpmc26 Mar 09 '17 at 06:39
  • 2
    Any decent IDE has the equivalent of "Find all references" to a method/class etc. Why has this not been mentioned yet? – user9993 Mar 09 '17 at 08:38
  • 5
    @user9993 it was mentioned earlier: reflection: *"The compiler can't be trusted if any form of meta-programming such as reflection is in play"*. Also, if it's an API or shared code, it might be being consumed in a different solution. – freedomn-m Mar 09 '17 at 08:51
  • 5
    "Fun" story: one time I was assigned to get a code-coverage tool working on a project compiled with MSVC. The code-coverage tool could not even parse certain files in the codebase, so I investigated. It turned out there were many files filled with hundreds of lines of template code, none of which was instantiated. That the templates weren't instantiated was clear from the fact that *the last line in every block was missing the concluding semicolon*. (MSVC doesn't even syntax-parse templates that aren't instantiated.) ... – Kyle Strand Mar 09 '17 at 23:52
  • 2
    ... One lesson is that sometimes you can get lucky and be able to determine with certainty that code really truly isn't used because syntactically it *can't* be compiled, which might be revealed (along with other warts and oddities) by compiling with a different compiler. But there are, uh...[other lessons](https://martinfowler.com/bliki/Yagni.html) I drew from the experience, as well. – Kyle Strand Mar 09 '17 at 23:54
  • 1
    @user9993: **Find references** _is_ worth mentioning, but the code could be an `if` with a condition we are (almost) sure is never satisfied, quite apart from the reflection issue. – PJTraill Mar 10 '17 at 09:15
  • 1
    @jpmc26 I have to disagree. The "maintenance burden" imposed by superfluous code is trivial, at least in my experience. What maintenance will have to be done on code that is never executed? – Michael J. Mar 10 '17 at 20:24
  • 6
    @MichaelJ. If you know it's not executed, you can safely delete it. If you don't know that, whenever you modify code in its vicinity, you have to maintain it. Just a month or so ago, I saw some code that looked broken. I had to trace through every reference to the method to figure out that arguments that triggered the broken block were never passed in. If executed, it would have associated some related records with ID 0 because it inserted them *before* creating the main record and fetching the new ID. That was a complete waste of a few hours, but I couldn't just have a bug there, either. – jpmc26 Mar 10 '17 at 20:44
  • 1
    @jpmc26 I suppose that "burden" is a subjective term. I do not see wasting several hours examining superfluous code as particularly burdensome. I know that I would spend far more time than that if I were wrong and deletion caused a production stoppage at a customer site. – Michael J. Mar 10 '17 at 21:10
  • 9
    @MichaelJ. My point is that it *shouldn't* have been there in the first place. And if I leave it, the situation is worse: now some other developer down the line has to investigate it as well. The longer it stays, the more time gets spent understanding it. This is the cost of superfluous code. – jpmc26 Mar 10 '17 at 21:27
  • 1
    @MichaelJ. My comment above also indicates a way in which superfluous code can be burdensome: it can make codebase-cleanup more difficult, and a sufficiently warty codebase can sometimes prevent you from using certain tools. But more importantly, I'm shocked that you don't consider hours of pointlessly wasted time a "burden". – Kyle Strand Mar 11 '17 at 05:16
  • 4
    `git blame` to find out who added it and why, might also be worth seeing if it is covered by tests as that could explain the purpose of it. – Mark K Cowan Mar 11 '17 at 13:28
  • 1
    For what it is worth, today I took a final pass through code prior to release to clean up and ran across a method my IDE says isn't used, and to the best of my recollection it is a remnant from an early feature that got pulled. I added a line to log if it is called and moved on and wouldn't have thought about it except I recalled this thread. – Thomas Carlisle Mar 11 '17 at 18:48
  • 2
    It is things like this that make extremely strict languages like Haskell look very attractive. In theory, a well managed, highly pure functional system would allow you to identify large swathes of the application's state space that are either impossible to enter, or that can only be entered via scenarios that are impossible or preventable out in the real world. (edit) I mean to say, a stricter language would give a higher resolution view of the outline of the state space, thereby clearly excluding more entities from the necessary part of the codebase. – Dan Ross Mar 12 '17 at 02:18
  • This question has the wrong title. The title asks how to delete the code, but the body is about how to identify whether the code is safe to delete in the first place. – underscore_d Mar 12 '17 at 13:56
  • 1
    An anecdote: I recently deleted a property from a JSON blob that's "broadcast" to various places. After grepping the entire codebase and not finding anything that used that property, I deleted it in the name of code cleanup. I then discovered that some webpages defined in the *database* used that property. – PotatoEngineer Mar 12 '17 at 18:53
  • @PaulMarshall "web pages defined in the *database*" ... This is definitely a design decision to avoid. – jpmc26 Mar 12 '17 at 22:21
  • 1
    After you have investigated the code, add what you have learned as comments for the next reader. This is even if you do not change anything! The code is not documented well enough (or you wouldn't have had to investigate) and you should be a good boy scout and leave it in a nicer state than when you found it. – Thorbjørn Ravn Andersen Mar 13 '17 at 08:57
  • @Kyle Strand You misread. I did not say that the effort was not burdensome. I said it was not _particularly_ burdensome, and I still think you are over estimating the time you'll spend maintaining superfluous code. . By all means, take whatever risks you think appropriate, but I've _rarely_ seen a case where removing a _potential_ maintenance burden justifies the risk of causing a production stoppage at a customer site. – Michael J. Mar 13 '17 at 12:58
  • @MichaelJ. "Not particularly burdensome" versus "not a burden" seems a bit like splitting hairs to me, but your point is taken. For what it's worth, we ultimately abandoned the project to use the code-coverage tool, and there have been no further efforts to generate useful metrics about the codebase. Of course, the templates that couldn't be compiled weren't the entire reason for deciding that the costs outweighed the benefits, but that was part of it. – Kyle Strand Mar 13 '17 at 20:10

20 Answers20

109

In my perfect fantasy world where I have 100% unit test coverage I would just delete it, run my unit tests, and when no test turns red, I commit it.

But unfortunately I have to wake up every morning and face the harsh reality where lots of code either has no unit tests or when they are there can not be trusted to really cover every possible edge case. So I would consider the risk/reward and come to the conclusion that it is simply not worth it:

  • Reward: Code is a bit easier to maintain in the future.
  • Risk: Break the code in some obscure edge case I wasn't thinking about, cause an incident when I least expect it, and be at fault for it because I had to satisfy my code quality OCD and make a change with no business value perceivable to any stakeholder who isn't a coder themselves.
Philipp
  • 23,166
  • 6
  • 61
  • 67
  • 253
    Having observed the state of large-ish software projects where engineers, for ten to twelve years, had mostly followed the "not worth it" approach to removing likely superfluous code, I *cannot* recommend this approach. – njuffa Mar 07 '17 at 15:48
  • 125
    Also, unit tests tell you nothing about whether this code is actually used in production. The code could be perfectly tested, but still not used in production, therefore superfluous. – knub Mar 07 '17 at 16:15
  • 8
    In my experience the state of such projects is never due to a "not worth it" approach to fixing the code after the fact, it's always due to bad code being written in the first place. It is a mistake to write bad code, but it's also a mistake (a more advanced mistake) to think that cleaning code is always worth it. – Weyland Yutani Mar 07 '17 at 17:46
  • 3
    @knub's comment happened to me on a new project that did (and a year and a half later, still does) have 100% coverage - the function was well-tested, but no longer used a couple months after it was added. – Izkata Mar 07 '17 at 18:08
  • 42
    @Weyland Yutani That does not match my experience at all. Most of the likely superfluous code I have seen was sufficiently well written and perfectly reasonable given software specifications, hardware platforms, or operating system versions in existence a decade prior, needed to work around bugs or limitations in the tool chain or hardware available earlier, etc. – njuffa Mar 07 '17 at 18:15
  • 3
    I really like this answer because it answers the question while highlighting that business value versus risk needs to be part of our everyday mind-set, and clean code isn't a high priority from a business perspective. Yes, clean code leads to quicker enhancements and improves time to market. Yes, clean code can lead to reduced risk when tasked with modifying the system. But a "problem" such as this has business risk high written all over it, and business value is very little. – Thomas Carlisle Mar 07 '17 at 23:47
  • 21
    Answer misses one of the most critical and important benefits of going ahead and making the change: learning a thing. Having learned it, you can add structure (comments, tests) to help maintenance devs later. Choosing not to change something because you don't know the consequences is cargo cult programming, even if that choice is not to delete something. – kojiro Mar 08 '17 at 11:15
  • 6
    njuffa is correct, but there's another option here that this answer misses. Write the unit tests, use this an opportunity to edge closer to your "perfect world". People have the attitude that making the code they touch better isn't worth it (because it's risky, time consuming, doesn't deliver value now etc) are sabotaging their ability to quickly deliver working stuff in the long run. – Nathan Cooper Mar 08 '17 at 13:15
  • 7
    @njuffa There is a survivor bias issue; only those projects that continue to exist are the ones you get to suffer under. Consider that the ones that followed your advice may have gone under due to spending too much time paying down technical debt, and not enough time developing new features... Rock, meet hard place. – Yakk Mar 08 '17 at 13:41
  • 2
    people want there to be a solution just like they want UFOs to exist. Sometimes there just isn't a solution to a terrible codebase other than living with it. A series of bad decisions has lead to a dead end. Superfluous code isn't even an issue really in those circumstances, it's the code that isn't superfluous that is a far bigger problem. – Weyland Yutani Mar 08 '17 at 16:05
  • 3
    @Yakk I have seen the actual business cost of not cleaning up: Lower productivity and more customer-visible bugs as even the most senior and *highly paid* engineers struggle to understand and refactor the code. If the risk of ripping out likely unused code is perceived excessive, the software is probably in a bad place already. I agree with Nathan Cooper: Take the opportunity to improve test environment and design documentation. Strive to become pound-wise instead of penny-wise and pound-foolish. – njuffa Mar 08 '17 at 16:31
  • 2
    @njuffa I agree! There are substantial costs down the road. Assuming that your software survives another 10 years, investing in making the code base better 10 years from now is *always* a great plan. But the software being around in 10 years is a function of how much you invest on 10-year horizons and how much you invest in immediate payoff.Everyone gets to feel the pain of a limping code base that you wish had more polish; nobody experiences an awesome codebase that didn't sell and the company went under. It can actually make perfect business sense to accumulate code debt and make money now. – Yakk Mar 08 '17 at 16:42
  • 1
    @njuffa Sadly, the code debt is an *invisible* debt. Such invisible debt doesn't easily show up on decision makers balance sheets; so you will have a systematic bias towards generating *more debt* than is wise for a given business. Off the books debt is free in a spreadsheet... Engineers pushing to keep that debt down is good for a business; but so is shipping something tomorrow. ;) – Yakk Mar 08 '17 at 16:44
  • 1
    @Yakk I understand about shipping on time, priding myself in always delivering to (or ahead of) deadlines. I also understand making money now (incl. startups). But I am also the guy who always tried (retired now) to rip out obsolete code quite aggressively (support for a platform discontinued? remove the associated code, now), because I observed the high cost (including opportunity cost) associated with keeping gunk in. Software tends to live a lot longer than most people anticipate, and the ten or twelve years I mentioned earlier are on the low side; some projects contain code from the 1980s! – njuffa Mar 08 '17 at 18:11
  • 3
    This answer doesn't make any sense. If you have a method `X` and a test on `X`, running that test would not provide any indication whether `X` is in use. Running a unit test on function `Y` that uses `X` wouldn't either, because `X` would be mocked or isolated away. There may be other automated tests that could help, but not *unit* tests. – John Wu Mar 08 '17 at 21:59
  • @JohnWu I was interpreting the answer as asking for unreachable code *inside a method*. – Philipp Mar 08 '17 at 22:06
  • Hmm... forgive me if I'm being a little stupid here. If the OP has a unit test that exercises the code, then the question changes from "is this code in use?" to "is this test case valid?" which, again, cannot be answered by unit testing. If the OP does not have a unit test that exercises the code, then the question is "should I add a test case?" which again unit testing itself cannot answer. – John Wu Mar 08 '17 at 22:23
  • "In my perfect fantasy world where I have 100% unit test coverage..." By definition, at least one test would fail in this case. – jpmc26 Mar 09 '17 at 02:06
  • 3
    @Yakk There is business value, and I think its highly misleading (and incorrect) that working to keep code clean always means the business goes under. Technical debt is only invisible because its impacts are not being correctly quantified. My current employer knows the technical debt has caused 80 hours (so far) of work which has put the new features behind schedule. The choice is pay a little along the way or pay in one big lump sum (usually in the middle of big important project). – Andy Mar 09 '17 at 02:14
  • 3
    @andy I am saying that unlimited investment in paying down technical debt can kill a company; and if lots of old code bases that survive are a mess, it does generate *some* evidence that immediately shipping features help companies stay alive better than pristine code does. The evidence isn't *strong*, nor convincing. My bias as a programmer is toward awesome and clean code (that is what I swim in!); but I must consider that *possibly* it isn't worth it for every company. On the other hand, there is evidence that _under investment in code quality_ may be "natural" and an error. – Yakk Mar 09 '17 at 04:08
  • 1
    @Yakk I don't think anyone here was arguing for unlimited time spent paying down technical debt, so you're arguing against a strawman. The entire point of calling it technical debt is that it works similar to actual debt, in that having small, responsible debt CAN have positive short term impacts, provided you make a plan to pay it off. A code base which is a huge mess is the same as high credit card debt which isn't being paid or perhaps only paying the minimum balance. – Andy Mar 10 '17 at 00:46
  • 2
    While this approach is sounds perfectly reasonable when looking at a single method, the effect of not removing dead code is cumulative. The project I'm currently looking at I'd estimate is 30-50% dead code (when dead code starts calling other dead code, its almost impossible to tell what is and isn't used any more). That's a huge amount of time spent maintaining, understanding and debugging code which offers no business value. – Justin Mar 10 '17 at 10:01
  • 1
    @Kojiro I have to disagree strongly. It is not cult programming to not tear a piece of code out of a production code base until you are sure of the consequences. In my organization, going ahead and making the change and being wrong about it not being used will most likely get a developer fired.... and possibly his boss. – Thomas Carlisle Mar 11 '17 at 18:55
  • 1
    Reward: The code is set in stone so you don't have to do any work. Risk: This obscure code is a carry over from an early system that is just waiting to cause havoc on this new system as soon as the goddess of luck decides to run it. That this answer should be so well received makes me weep for the state of my field. The idea of "[first, do no harm](http://www.health.harvard.edu/blog/first-do-no-harm-201510138421)" leaves out a lot. Clutter in the code is just flat out dangerous. Clutter begets clutter. Just leaving it is as bad as having written it. Learn what it does and when it does it. – candied_orange Mar 11 '17 at 20:06
  • Only the first sentence of this "answer" addresses the question, and it's not helpful at all. – user673679 Mar 12 '17 at 15:19
  • @CandiedOrange Regardless of the merits of this answer, I think that "first to answer bias" and resulting voting (earlier answers tend to list higher and attract more votes) makes it difficult to compare the true value of answers based on their vote count, and the vote count can actually be quite misleading in this regard. I think this is a significant area of potential improvement for Stack Exchange. – Bradley Thomas Mar 12 '17 at 19:47
  • 1
    @BradThomas that there exist 116 developers who are content to leave code that they don't understand disturbs me. Simply not understanding it is forgivable. But reacting to that by just walking away is unprofessional. We write software because we want it to be easy to change. If we wanted unchanging behavior we've have hired electrical engineers to do it faster with chips. Code that can't be touched is dead. Systems with such code are at their end of life. If you really can't figure it out start replacing the system. Only tolerate understandable code. – candied_orange Mar 13 '17 at 03:42
  • 1
    Why is this the most voted answer? This is exactly how projects get slowly stuffed of incomprehensible edge cases and unmaintenable architectures, creeping towards an agonizing death several years later, unless some poor soul takes on itself the duty of cleaning the mess left by scared code monkeys that added code to "just make it work". – BgrWorker Mar 13 '17 at 08:08
  • I really want to ride around in a driverless car some day. But not if this is the way it's coded. – candied_orange Mar 14 '17 at 05:32
  • @ThomasCarlisle If making a mistake gets you fired then it's good to get out fast. Cultures like that are toxic. Relatively few legitimate mistakes warrant that kind of response. – Dave Newton Mar 20 '17 at 11:04
  • @DaveNewton The point I tried to make is simply that developers tend to put code cleanliness as a high priority and lose sight of the risk that is inherent in removing suspected dead code. I like this answer because it highlights the need to pause and the very real reality that the test coverage you think is there might not be as solid as you thought. This all becomes a recipe for disaster when the application is updated and features break that shouldn't have been touched and therefore don't get as much testing as they should, and you have to come up with a good explanation in business terms. – Thomas Carlisle Mar 21 '17 at 21:37
86

There are two halves to this process. The first is confirming that the code is indeed dead. The second is comprehending the costs of being wrong and making sure they are properly mitigated.

Many answers here have great solutions to the former half. Tools like static analyzers are great for identifying dead code. grep can be your friend in some cases. One unusual step I often take is to try to identify what the code’s original purpose was. It’s a lot easier to argue “X is no longer a feature of our product, and code segment Y was designed to support feature X” than it is to say “I don’t see any purpose for code segment Y.”

The second half is a key step to breaking any gridlock over whether you should remove code. You need to understand what the implications are of getting the answer wrong. If people are going to die if you get the answer wrong, pay attention! Maybe it’s a good time to accept that code cruft develops over time and instead try not to write more cruft yourself. If people aren’t going to die, ask yourself how forgiving your users are. Can you send them a hotfix if you broke something and maintain your customer relations? Do you have a Q&A team that’s paid to find issues like this? These sorts of questions are essential for understanding how certain you must be before you hit the delete key.

In the comments, rmun pointed out an excellent phrasing of the concept of understanding the original purpose of the code before removing it. The quote is now known as Chesterton’s Fence. While it is too large to be quoted directly in a comment, I think it deserves to be properly quoted here:

In the matter of reforming things, as distinct from deforming them, there is one plain and simple principle; a principle which will probably be called a paradox. There exists in such a case a certain institution or law; let us say, for the sake of simplicity, a fence or gate erected across a road. The more modern type of reformer goes gaily up to it and says, “I don’t see the use of this; let us clear it away.” To which the more intelligent type of reformer will do well to answer: “If you don’t see the use of it, I certainly won’t let you clear it away. Go away and think. Then, when you can come back and tell me that you do see the use of it, I may allow you to destroy it.

SusanW
  • 1,035
  • 10
  • 14
Cort Ammon
  • 10,840
  • 3
  • 23
  • 32
  • 15
    Chesterton's Fence assumes discovery comes purely by thinking, and offers no recourse–a rather Ancient-Greek philosophy. The scientific method is more modern–devise a controlled experiment to determine the consequences of removing the structure. To be sure, I wouldn't nonchalantly propose this experiment be done in production. But if there is no alternative, and the cost of trial in production is very high, well, I would rather quickly leave a workplace which offered me no safe means to learn why some code exists–the personal risk at such a job is too high. – kojiro Mar 08 '17 at 12:37
  • 15
    @kojiro That is very true. I like to think that Chesterton would be okay if I, as a "modern reformer" came by and said "I don't know why this fence is here, I admit it. But I'd like to paint it red to see if that provides me any new information," Chesterton would probably be happy with it. – Cort Ammon Mar 08 '17 at 14:13
  • 42
    The VMS OS kernel mode clock decoding method SYS$ASCTIM contains a line of code that helps correct for the sidereal drift of the vernal equinox. None of DEC's unit tests exercises this code. It ran once -- correctly -- on 2000-02-28 at 23:59:59:999. It will not run again until year 2400. Please do not delete it. – A. I. Breveleri Mar 08 '17 at 21:40
  • 3
    @kojiro You skipped the step of exhausting other reliable sources of information before you conduct an experiment. ;) Also, our "modern scientific method" also demands you form a hypothesis to test. That part, at the least, does require going away to think first. – jpmc26 Mar 09 '17 at 02:12
  • 3
    I really love the quote of Chesterton's Fence. With code, thinking *will* produce an answer. Either the code is dead, or it's alive, and you CAN arrive at a conclusion as to which is right by studying the code. **Prove** to yourself that the code is superfluous, and then remove it. If you can't prove it's dead, leave it. Code isn't magic, it's pure logic, and programming is one of the very few human endeavors where we *can* be sure about things (unlike science, which can only build models of reality). The real question, I guess, is how much time you're willing or able to invest in a proof. – Pascal Mar 09 '17 at 16:55
  • 13
    @A.I.Breveleri I should hope that there's a comment in the code itself explaining this, and not merely here on StackExchange :D – Kyle Strand Mar 09 '17 at 23:44
  • 11
    @KyleStrand What are you talking about? This *is* the documentation. Can you think of a better place to put crucial information so that other people find it than StackExchange? ;-) – Cort Ammon Mar 09 '17 at 23:46
  • 1
    @CortAmmon You are confused. (SO) Documentation is the documentation. It says so in the name. – Kyle Strand Mar 09 '17 at 23:57
  • 3
    @A.I.Breveleri: If that line of code really isn't exercised by any tests, I'd say that's a gap in their testing coverage that they should fix. If the code does what I assume it does (i.e. ensures that years divisible by 400 are correctly treated as leap years) then it shouldn't even be particularly hard to test. Still, I do get your point; it's fine for us here in the peanut gallery to argue that the solution is more tests, but the fact remains that very few if any real world codebases actually have 100% effective test coverage. – Ilmari Karonen Mar 11 '17 at 02:22
42

I also tend to grep for the function/class name in the code, which can give some additional benefits that a code analyzer might not, like if the name is mentioned in a comment or in a documentation file, or a script, for example. I run grep on the files in the source tree and store the result in a file; usually the result gives a condensed information: file name/path, line number, and the line where the name is encountered, which can give clues to where the function/class is called or mentioned without any semantic meaning (in contrast to a code analyzer), and regardless of the file extensions. Definitely not the ultimate solution but a nice addition to an analysis.

piwi
  • 486
  • 3
  • 7
  • 11
    I am not a downvoter, but what if the code section you suspect doesn't ever get run is not a complete function or class but a section of a method/function? – Tulains Córdova Mar 07 '17 at 17:01
  • 9
    Depending on the language this may not always be reliable - some languages allow method calls to be made dynamically. For example php: `$object->$variableMethod($arg1, $arg2);` – Dezza Mar 07 '17 at 17:20
  • 9
    @TulainsCórdova Then this is probably not a good solution, obviously. As for every potential solution, use it if it makes sense in the given context. But maybe `grep`ing the eg. function encompassing the code section can give clues as to how the function is used and therefore its content? – piwi Mar 07 '17 at 21:44
  • 7
    "like if the name is mentioned in a comment or in a documentation file, or a script" - Or if someone is using reflection to call the method (if using a high-level/OO language). Though still not necessarily 100% accurate, as some languages support some very obtuse ways of finding and invoking methods. – aroth Mar 08 '17 at 05:16
  • This is helpful if you have access to all possible code that could use the function. However, if the code is part of a library, how are you going to know that something else you can't search through won't break? – Kevin Shea Mar 08 '17 at 14:54
  • @Kev Well as answered to TulainsCórdova you use another solution, obviously... – piwi Mar 08 '17 at 15:16
  • @aroth `some very obtuse ways of finding and invoking methods` - like many Java annotation-based methods of deciding which methods to call. – Logan Pickup Mar 08 '17 at 21:32
29
  • Identify the code that looks dead (static analysis, etc).
  • Add a log message for every invocation of the allegedly dead code. It's easy with functions / methods; with static members like constants it's trickier. Sometimes you can just mark code as deprecated, and the runtime will log a message for you automatically. Leave the code otherwise intact.
    • Log a message when the dead module is loaded: most languages have a way to run static initialization at load time, which can be piggy-backed.
    • Make sure your log message includes a reasonable stack trace, so that you understand what called the allegedly dead code.
  • Run the altered code through all your test suites. Your test suites should also test for special times, like crossing a midnight, a turn of a quarter, a turn of a year, etc. Look at the logs, update your understanding of what's dead. Note that unit tests may specifically test the dead code, while no other unit tests and no integration tests touch it.
  • Run the altered code in production for a few weeks. Make sure every periodic process, like those once-a-month ETL cron jobs, are run during this period.
  • Look at the logs. Anything that was logged is not actually dead. The transitive closure of the call graph over the rest of the dead code is also potentially not dead, even if it was not invoked. Analyze it. Maybe some branches are safely dead (e.g. working with an API of a now-extinct service), while some are not yet. Maybe a whole module / class is only loaded for a constant defined in it, and you can easily disuse it.
  • Whatever is left is safely dead, and can be removed.
9000
  • 24,162
  • 4
  • 51
  • 79
  • There is a first time for everything, so just because code didn't run before doesn't mean it will never run, particularly if there is an execution path to it. That said, *deciding* to remove something that can execute is a different process from determining if it could run. –  Mar 07 '17 at 17:20
  • 7
    @nocomprende exactly what happens if this code is only used for an end of year function and you run your test is run Feb - Nov. You won't find the usage even though you profiled it for nearly a whole year. – Erik Mar 08 '17 at 20:42
  • @Erik: This is specifically mentioned. Mocking time to match special periods must be a part of normal test suites, too. – 9000 Mar 08 '17 at 21:44
  • 1
    @9000 I agree in theory but legacy systems are riddled with hidden dependencies that can foil this kind of testing. **For your testing example to work you need to know that there is some kind of temporal coupling.** If the temporal coupling is external to the code you're testing then you aren't going to see it in the code and know that temporal coupling is a factor. For example silo A's code is installed in the [GAC](https://en.wikipedia.org/wiki/Global_Assembly_Cache). Years ago silo B asked silo A to add a code path for a report they need run against silo A's data. cont. – Erik Mar 08 '17 at 21:56
  • 1
    @9000 cont. Now silo B's code has a temporal coupling to a "dead" code path in silo A's code that isn't apparent by just looking at silo A's code. How would you unit test for this temporal coupling since the temporal coupling is only in silo B's code without knowing to talk to silo B? Even then time isn't a factor in your code (silo A) so what would a temporal test look like? – Erik Mar 08 '17 at 22:03
  • @9000 my point was not about testing, testing can never reveal how code *could potentially* be used. My point was: if the code is never called, it cannot run. So in the situation where I had to do this, I stripped out methods that were never called. Simple and foolproof. Beyond that, you cannot be certain. We should be making systems simpler, not more complex and interconnected. And we should not be modifying 4 billion year old programs (DNA). –  Mar 09 '17 at 14:19
  • 2
    Anybody remember __Y2k__ panics? Some code was right in 2000 because it was wrong for 2100! We had a potential issue that could occur once in 100 years or even once in 400 years. – Steve Barnes Mar 11 '17 at 06:43
16

In addition to the existing answers mentioned you could also iteratively remove the code over several versions.

In the initial version you could do a deprecation warning with the code block still functioning. In the version after that you could remove the code block but leave an error message letting users the feature is deprecated and no longer available. In final version you would remove the code block and any messages.

This may help identify unforeseen functionality with no warning to end users. In the best case scenario the code really does nothing and all that happens is that the unneeded code is kept over several versions before being removed.

user3196468
  • 271
  • 1
  • 7
  • 7
    +1. I've seen so many bugs and customer-facing issues that could have been avoided if the developers (or their management) had been willing to break a project into two or three safe phases instead of trying to throw everything in at once to meet an arbitrary deadline before moving onto the next project. – ruakh Mar 08 '17 at 18:15
  • This answers the question asked in the title, but if you ask me, the question has the wrong title. The title asks how to delete the code, but the body is about how to identify whether the code is safe to delete in the first place. You definitely answer the former, but I'm not sure that's what the OP was really asking. – underscore_d Mar 12 '17 at 13:57
7

A lot of people are suggesting that the "safe" thing to do is to leave the code in if you can't prove it's unused.

But code is not an asset, it's a liability.

Unless you can explain why it's important and point to its tests, the "safer" alternative might very well be to delete it.

If you're still not sure, then at least make sure to add tests to exercise the zombie code.

Adrian McCarthy
  • 981
  • 5
  • 8
  • Step 1: Delete all code. Step 2: put back what is needed. Step 3: repeat. –  Mar 09 '17 at 14:26
  • 1
    @Adrian Can I draw your attention to Knight Capital? https://en.wikipedia.org/wiki/Knight_Capital_Group#2012_stock_trading_disruption - just in case you want to bolster your point with a reference to it. They basically imploded because some old code came back to life and kindly lost almost $500m for them. The ensuing inquiry focused on their release procedures, but the core problem was "dead functionality wasn't deleted". – SusanW Mar 13 '17 at 13:02
6

You can use feature toggles to change the execution path of your software to completely ignore the code in question.

This way you can safely deploy your change with the code not in use, and the toggle off. If you notice some major faults related to the code turn the toggle back on and investigate the possible path to it.

This approach should give you confidence if you see no problems over prolonged period of time as well as the ability to turn it back on live w/o a deployment. However an even better way would be to also apply extra logging and test coverage around the area in question which will provide more evidence whether it is used or not.

Read more about toggles here: https://martinfowler.com/articles/feature-toggles.html

Sash
  • 165
  • 4
6

Static analysis of course... and the nice thing is, you don't need any new tools; your compiler has all the static analysis tools you need.

Just change the name of the method (e.g. change DoSomething to DoSomethingX) and then run a build.

If your build succeeds, the method, obviously, isn't being used by anything. Safe to delete.

If your build fails, isolate the line of code that calls it and check to see what if statements surround the call to determine how to trigger it. If you can't find any possible data use case that triggers it, you can safely delete it.

If you are really worried about deleting it, consider keeping the code but marking it with a ObsoleteAttribute (or the equivalent in your language). Release it like that for one version, then remove the code after no problems have been found in the wild.

John Wu
  • 26,032
  • 10
  • 63
  • 84
  • 8
    This is ok advice for programming languages that don't have full [dynamic/late binding](https://en.wikipedia.org/wiki/Late_binding). However, for those that have, it's trickier. And, of course - code might be using the method even if it's never used in production. – eis Mar 08 '17 at 07:30
  • The question presupposes compile-time symbol resolution (*So you've found some code that looks superfluous. The compiler doesn't notice that.*). And even late bound function or objects are typically defined in two places, e.g. an interface or map file, so the build would still fail. – John Wu Mar 08 '17 at 09:48
  • 1
    Hmm... I'm intrigued, I don't think the latter you say is true. It's not for vanilla javascript (not precompiled), ruby, python or smalltalk for example, right? If you refer to stuff that doesn't exist, those error only at runtime. – eis Mar 08 '17 at 10:31
  • Maybe we disagree on what is mean by "binding" which to me indicates the resolution of a logical representation to a physical representation, e.g. a symbol to an address. Javascript functions are stored as tag/value pairs within the containing object, so any notion of binding seems non sequitur. – John Wu Mar 08 '17 at 20:46
  • 1
    Yes, we really disagree. The term has different meaning on languages that don't have full late binding (like I explained in the first comment). When you bind methods at runtime - in languages that can add and remove methods runtime, on the fly - you are using late binding in the way I understand it to be done. This is true in ruby/python/smalltalk-style languages, and there you don't have separate map files. Since especially ruby and python are very widely used, I disagree about your notion what it would "typically" mean. – eis Mar 08 '17 at 21:44
  • Fair enough. I definitely agree that compile time checks won't catch everything in every language (nothing would, really, except maybe fuzz testing), which is why I included a note about deprecating the function via metadata. – John Wu Mar 08 '17 at 21:56
4
  • I would start using a code analyzer to check whether this is dead code
  • I would start checking my tests and trying to reach the code. If I am not able to reach the code within a test case it might be dead code
  • I would remove the code and throw an exception instead. For one release the exception is activated, and in the second release the exception can be removed. To be safe, place an emergency flag (in Java, a system property) to be able to activate the original code, when a customer notices the exception. So the exception can be disabled and the original code can be activated in a production environment.
Peter Mortensen
  • 1,050
  • 2
  • 12
  • 14
Markus Lausberg
  • 149
  • 3
  • 6
    -1. It is absolutely not recommendable to make these changes in productive code, safety flag or not. Only when one is *sure* the code in question isn't used should it be removed in productive code. That kind of messing-around is exactly why there should always be different productive and testing environments. – Johannes Hahn Mar 07 '17 at 16:18
  • 3
    This will work in fantasy when test coverage is nearly 90%, but what if not and you have a project with 100.000 lines of code. Thats why i described to try testing the code and if no test is able to reach the code... it might be dead. – Markus Lausberg Mar 08 '17 at 06:40
  • IMHO If you are unsure enough to hold yourself from removing the code entirely - you should not be throwing exceptions in production - logging suggested by the OP is a much better alternative, achives the same, and you can get diagnostics instead of relying on customers complaining – Sebi Mar 08 '17 at 19:15
  • @JohannesHahn I think you mean, "production code," which is code that runs in a production environment. – jpmc26 Mar 09 '17 at 02:18
  • @jpmc26 Yes, of course. – Johannes Hahn Mar 09 '17 at 08:18
  • @Markus Lausberg: Surely whether 100'000 lines of code can be understood or not depends very much on it's quality, e.g. how modularized it is etc. If there is a good design and the codebase is split into modules accordingly, chances are you won't have to look at 100'000 lines but maybe at 1000 lines, or 5000 lines, to understand interactions with any piece of code you're interested in. Test coverage won't even come into play. – Pascal Mar 09 '17 at 17:20
3

Removing unreachable code

In a principled statically typed language, you should always know whether the code is actually reachable or not: remove it, compile, if there is no error it was not reachable.

Unfortunately, not all languages are statically typed, and not all statically typed languages are principled. Things that could go wrong include (1) reflection and (2) unprincipled overloading.

If you use a dynamic language, or a language with sufficiently powerful reflection that the piece of code under scrutiny could potentially be accessed at run-time via reflection, then you cannot rely on the compiler. Such languages include Python, Ruby or Java.

If you use a language with unprincipled overloading, then merely removing an overload could simply switch the overload resolution to another overload silently. Some such languages allow you to program a compile-time warning/error associated with the usage of the code, otherwise you cannot rely on the compiler. Such languages include Java (use @Deprecated) or C++ (use [[deprecated]] or = delete).

So, unless you are very lucky to work with strict languages (Rust comes to mind), you may really be shooting yourself in the foot by trusting the compiler. And unfortunately test suites are generally incomplete so of not much more help either.

Cue the next section...

Removing potentially unused code

More likely, the code is actually referenced, however you suspect that in practice the branches of code that reference it are never taken.

In this case, no matter the language, the code is demonstrably reachable, and only run-time instrumentation can be used.

In the past, I've successfully used a 3-phases approach to removing such code:

  1. On each branch suspected NOT to be taken, log a warning.
  2. After one cycle, throw an exception/return an error upon entering the specific piece of code.
  3. After another cycle, delete the code.

What's a cycle? It's the cycle of usage of the code. For example, for a financial application I would expect a short monthly cycle (with salaries being paid at the end of the month) and a long yearly cycle. In this case, you have to wait at least a year to verify that no warning is ever emitted for the end-of-year inventory could use code paths that are otherwise never used.

Hopefully, most applications have shorter cycles.

I advise putting a TODO comment, with a date, advising on when to move on to the next step. And a reminder in your calendar.

Matthieu M.
  • 14,567
  • 4
  • 44
  • 65
  • 1
    Actually, in C++ you can mark your suspect overload as `[[deprecated]]` to identify sites that call that version; then you can inspect them to see if their behaviour will change. Alternatively, define your overload as `= delete` - in this case, you will need to explicitly cast arguments to use one of the other versions. – Toby Speight Mar 08 '17 at 11:15
  • @TobySpeight: That's true, and a good alternative too. Let me edit that in. – Matthieu M. Mar 08 '17 at 11:34
  • "Doctor, it hurts when I do this." "Well *don't do that!*" Don't use approaches which make the code unmanageable. –  Mar 09 '17 at 14:23
2

Removing code from production is like cleaning house. The moment you throw away an object from the attic, your wife will kill you the next day for throwing away her great grand mother's third nephew's homecoming gift from their neighbor who died in 1923.

Seriously, after cursory analysis using various tools that everyone has already mentioned, and after using the stepped deprecation approach already mentioned, keep in mind that you may be gone from the company when the actual removal takes place. Letting the code remain and have its execution logged and ensured that any alerts of its execution are definitely communicated to you (or your successors and assigns) is essential.

If you don't follow this approach, you're likely to be killed by your wife. If you keep the code (like every keepsake) then you can rest your conscience in the fact that Moore's law comes to your rescue and the cost of junk disk space used by the code that feels like a "rock in my shoe", reduces every year and you don't risk becoming the center of multiple water cooler gossips and weird looks in the hallways.

PS: To clarify in response to a comment.. The original question is "How do you safely delete.." of course, Version Control is assumed in my answer. Just like digging in the trash to find the valuable is assumed. No body with any sense throws away code and version control should be part of every developer's rigor.

The problem is about code that may be superfluous. We can never know a piece of code is superfluous unless we can guarantee 100% of the execution paths do not reach it. And it's assumed that this is a large enough piece of software that this guarantee is next to impossible. And unless I read the question wrong, the only time this whole thread of conversation is relevant is for cases during production where the removed code might get called and hence we have a runtime/production issue. Version control does not save anyone's behind due to production failures, so the comment about "Version Control" is not relevant to the question or my original point, i.e. properly deprecate over an extremely long time-frame if you really have to, otherwise don't worry because superfluous code adds relatively a very small cost in disk space bloat.

IMHO, the comment is superfluous and is a candidate for removal.

LMSingh
  • 277
  • 2
  • 5
  • 12
    If I had the equivalent of *version control*, my wife's objection will be easily reversed. So too with deleting code: it's not a killing offense. The repository is *never* thrown away. – JDługosz Mar 08 '17 at 08:04
  • I thought that Object Oriented programming was supposed to narrow the scope in which a method could be called. So it should lead to code that is more easily understood and managed, right? Otherwise, why are we doing it that way? –  Mar 09 '17 at 14:25
  • @nocomprende Nowhere in this thread is it indicated that the discussion is limited to OOP design/languages only. I'm not sure why you think that's relevant. – underscore_d Mar 12 '17 at 14:07
1

Maybe the compiler does notice that, it just doesn't make a fuss.

Run a build with full compiler optimizations, geared towards size. Then delete the suspect code, and run the build again.

Compare the binaries. If they're identical, then the compiler has noticed and silently deleted the code. You may delete it from source safely.

If the binaries are different... then it's inconclusive. It could be some other change. Some compilers even include date and time of compilation in the binary (and maybe it can be configured away!)

Emilio M Bumachar
  • 791
  • 4
  • 6
  • 1
    Not all languages are compiled, not all compilers have optimisation and not all optimisations are equal, many do not remove un-invoked code just in case that code is there for a reason. If the code lives in a shared library/DLL it will not be deleted. – Steve Barnes Mar 11 '17 at 06:50
  • 1
    @SteveBarnes Well yeah, if you're not doing LTO and static-linking everything, you're in for some pain. That's a given. – Navin Mar 13 '17 at 11:21
1

I've actually recently come across this exact situation, with a method called "deleteFoo". Nowhere in the entire codebase was that string found other than the method declaration, but I wisely wrote a log line at the top of the method.

PHP:

public function deleteFoo()
{
    error_log("In deleteFoo()", 3, "/path/to/log");
}

It turns out that the code was used! Some AJAX method calls "method:delete,elem=Foo" which is then concatenated and called with call_user_func_array().

When in doubt, log! If you've gone sufficient time without seeing the log filled, then consider removing the code. But even if you remove the code, leave the log in place and a comment with the date to make finding the commit in Git easier for the guy who has to maintain it!

dotancohen
  • 1,001
  • 1
  • 8
  • 19
0

Use grep or whatever tools you have available first, you might find references to the code. (Not originally my instruction/advice.)

Then decide if you want to risk removing the code, or if my suggestion could come to use:

You could modify the function/block of code to write that it has been used to a log file. If no such message is "ever" recorded in the log file, then you can probably remove the logging code.

Two problems:

  1. Getting the log from other users. Perhaps you can set a global flag that will crash the program on exit and kindly ask the user to send you the error log.

  2. Tracing the caller. In some high level languages (eg. Python) you can easily get a traceback. But in compiled languages you'll have to save the return address to the log and force a core dump on exit (point 1).
    In C, this should be quite simple: use a conditional block (eg. #ifdef ... #endif) to only use this when you know it'll work (and have tested that it does), and simply read the return address from the stack (may or may not require inline assembly language).

Saving the arguments in the log file may or may not be useful.

Oskar Skog
  • 191
  • 2
  • 8
0

If you know what the code surrounding it does, test it to see if it breaks. This is the simplest and most cost-effective way to refactor a piece of code you're working on, and should be done anyway as a matter of developing any change to the code you're working on in the first place.

If, on the other hand, you're refactoring a piece of code where you do not know what it does, do not remove it. Understand it first, then refactor it if you determine it to be safe (and only if you can determine that the refactoring would be a proper use of your time).

Now, there might be some circumstances (I know I've run into it myself) where the code that is 'dead' is actually not connected to anything. Such as libraries of code developed by a contractor that never actually got implemented anywhere because they became obsolete immediately after the app was delivered (why yes, I do have a specific example in mind, how did you know?).

I personally did wind up removing all of this code, but it is a huge risk that I do not recommend taking on lightly - depending on how much code this change could potentially impact (because there's always the potential that you've overlooked something) you should be extraordinarily careful and run aggressive unit tests to determine if removing this ancient legacy code will break your application.

Now all of that being said, it probably isn't worth your time or sanity to try and remove code like that. Not unless it's becoming a serious issue with your application (for example, not being able to complete security scans because of application bloat...why yes I DO still have an example in mind), so I wouldn't recommend it unless you reach such a situation where the code has truly started to rot in an impactful way.

So in short - if you know what the code does, test it first. If you don't, you can probably leave it alone. If you know you can't leave it alone, dedicate your time to test the change aggressively before implementing the change.

Zibbobz
  • 1,522
  • 3
  • 13
  • 20
0

My approach, which I always assumed to be industry standard in this case, weirdly hasn't been mentioned so far:

Get a second pair of eyes.

There are different kinds of "unused code" and in some cases deleting is appropriate, in other cases you should refactor it, and in yet other cases you run away as far as you can and never look back. You need to figure out which one it is, and to do so you best pair up with a second - experienced! - developer, one who is very familiar with the code base. This significantly reduces the risk for an unnecessary mistake, and allows you to make the necessary improvements to keep the code base in a maintainable state. And if you don't do this, at some point you run out of developers who are very familiar with the code base.

I've seen the logging approach too - to be more exact, I've seen the logging code: even more dead code which was left in there for 10 years. The logging approach is quite decent if you're talking about removing entire features, but not if you're just removing a small piece of dead code.

Peter
  • 3,718
  • 1
  • 12
  • 20
0

The simple answer to your question is you can't safely delete code you don't understand, which includes not understanding how it gets called. There will be some risk.

You will have to judge how best to mitigate that unavoidable risk. Others have mentioned logging. Logging can of course prove that it is used, but can't prove that it isn't. Since the major problem with dead code is that it gets maintained, you might be able to get away with adding a comment saying that it is suspected dead code, and not to do any maintenace on it.

If the code is under source control, you can always find out when it was added and determine how it was called then.

Ultimately, you either understand it, leave it alone, or take a chance.

jmoreno
  • 10,640
  • 1
  • 31
  • 48
0

In case the developer of the code in question is still available, review the code removal with him. Also, read comments in the code.

You will get the proper explanation, why this code has been added and which function does it serve for. It can easily be support for the specific use case, or you even may not have enough expertise to judge if it is really not required. In extreme case, one may remove all free statements from a C program and fail to notice anything wrong (it may take few minutes to run out of memory).

It is really a bad culture when the author of the code sits next to you, yet you see no reason to talk because you are sure he just writes bad code, and yours is all so perfect. And, of course, he just writes random words in the comments, no need to waste time reading.

One of the most important reasons to write comment in the code is to explain WHY it has been implemented this way. You may disagree with the solution, but you need to take the problem into consideration.

Discussion with the author may also reveal that the code is the historical remnant of something removed or never finished, so is safe to delete.

h22
  • 905
  • 1
  • 5
  • 15
-1

I strongly agree with Markus Lausberg's first point: The code should definitely be analyzed with the help of tools. Ape brains are not to be trusted with this kind of task!!

Most standard programming languages can be used in IDEs and every IDE that's worth being called that has a "find for all usages"-feature which is exactly the right tool for this kind of situation. (Ctrl+Shift+G in Eclipse for example)

Of course: Static analyzer tools are not perfect. One must be aware of more complicated situations where the decision that the method in question is being called happens only at runtime and no sooner (calls via Reflection, calls to "eval"-functions in scripting languages etc).

Johannes Hahn
  • 328
  • 1
  • 10
  • 5
    Maybe ape brains should not be writing code? "*Untouched by human hands*" used to be a marketing phrase. I always imagined gorilla hands doing the work. –  Mar 07 '17 at 16:44
  • 3
    (Ape brains wrote the tools too) (Shhh! You'll ruin the story!) –  Mar 07 '17 at 16:46
  • 1
    I think you're both missing my point. I clearly said that there is no guarantee that static code analysis will find every invocation of a method or class. It is, as it says on the tin, a static analysis. It cannot and should be not be expected to find dynamic invocations via reflection, eval() etc. My point is that static code analysis should be considered a necessary prerequisite for deleting a piece of code. And the bigger the project is the less likely it becomes that a human brain is even capable of doing that analysis at all let alone doing it correctly... – Johannes Hahn Mar 09 '17 at 14:23
  • 1
    ... static code analysis is tedious, repetitive work which is best done by a computer. Exactly the kind of task that can be reliably executed by computers but is notoriously unreliable when executed by an ape brain. The latter should only be used for things a computer cannot do like finding those tricky reflections and eval-calls. – Johannes Hahn Mar 09 '17 at 14:26
  • So, the ape brains do the tricky stuff and tools do the simple stuff that ape brains could not be trusted with? How complex are the tools? Who created those? Can we make a software project so heavy that we cannot lift it? Software got us to the moon 60 years ago. How have things improved since then? (Be careful with units. *Doh!*) –  Mar 09 '17 at 14:29
  • 1
    Again, missing the point. Even if the tools aren't perfect because they were written by ape brains (and I'm not even sure if I should grant that! Computing static call hierarchies isn't that difficult), they are still far superior to manual task execution. This isn't an all-or-nothing-kind of situation. A 99% reliable tool is still preferable to a brain that maybe starts with 99% percent but quickly falls to much lower rates after the first thousand lines of code. – Johannes Hahn Mar 09 '17 at 14:34
-1

Two possibilities:

  1. You have 99%+ test coverage: Delete the code and be done with it.
  2. You don't have trustworthy test coverage: Then the answer is to add a deprecation warning. I.e., you add some code to that place that does something sane (log a warning to some easily visible place that does not conflict with day-to-day-usage of your application, for example). Then let it simmer for a few months/years. If you never ever found any occurence of it, replace the deprecation by something that throws an exception. Let that stand for a while. Finally, remove everything completely.
AnoE
  • 5,614
  • 1
  • 13
  • 17
  • 2
    this doesn't seem to offer anything substantial over points made and explained in prior 17 answers. Both coverage and deprecation warning were discussed over there already – gnat Mar 09 '17 at 20:53
  • @gnat, you are right. When I first scanned the answers, for some reason I thought I'd seen several ones that at the top that did not shortly and succinctly get to these two point. In the middle of the answers there are some like that. – AnoE Mar 10 '17 at 07:49