Forgot your password?
typodupeerror
Programming

What Are the Unwritten Rules of Deleting Code? 384

Posted by samzenpus
from the best-practices dept.
Press2ToContinue writes "I came across this page that asks the question, 'what are the unwritten rules of deleting code?' It made me realize that I have seen no references to generally-accepted best-practice documents regarding code modification, deletion, or rewrites. I would imagine Slashdot's have come across them if they exist. The answers may be somewhat language-dependent, but what best practices do Slashdot's use when they modify production code?"
This discussion has been archived. No new comments can be posted.

What Are the Unwritten Rules of Deleting Code?

Comments Filter:
  • The more..... (Score:5, Insightful)

    by phantomfive (622387) on Monday January 07, 2013 @03:06AM (#42501987) Journal
    The more you can delete, the better.
    • Re:The more..... (Score:5, Informative)

      by rwven (663186) on Monday January 07, 2013 @03:33AM (#42502111)

      Yeah, I have no qualms about deleting code. There are others I've seen who just comment out massive blocks of code and leave them there....for me to eventually find and delete myself, apparently. Version control is there for a purpose. If you need something back...it's there.

      • Re:The more..... (Score:5, Insightful)

        by wmac1 (2478314) on Monday January 07, 2013 @03:58AM (#42502245)

        I comment the code and leave it there for at least one minor version. If things are all right, I'll remove them. The code is in source control anyway.

        • Re:The more..... (Score:5, Insightful)

          by mwvdlee (775178) on Monday January 07, 2013 @04:14AM (#42502339) Homepage

          Why not just delete it right away? Commenting out code in version control just creates more changes.

          • Re:The more..... (Score:5, Insightful)

            by Anonymous Coward on Monday January 07, 2013 @04:41AM (#42502491)

            If everyone uses the system "comment out first, remove after it has shown to work", seeing commented out code means that the code is still being reviewed. That might be valuable information.

            • Re: (Score:3, Insightful)

              by Whiteox (919863)

              I always keep in mind that the code I'm working on will be assembled, and the assembly is smaller and tighter with assembly oriented coding. So I do comment out working code and replace it with an alternate if I think it could be more efficient.

          • Re:The more..... (Score:5, Interesting)

            by Pieroxy (222434) on Monday January 07, 2013 @04:46AM (#42502521) Homepage

            Once it's not in the file anymore it's out of your attention span. Leaving the code there for a revision or two allow anyone looking into the file to be instantly aware that something has happened very recently, which is usually what the person is looking for.

            Depending on your language, putting a "TODO" marker allows for easy and quick cleanup afterwards.

        • most of the time I want to delete old code when I've written something newer to replace it. I keep the old code there for easy reference, until I'm confident the new code works at least as reliably as the old one.
        • Re:The more..... (Score:5, Insightful)

          by tknd (979052) on Monday January 07, 2013 @06:13AM (#42502963)

          If you're going to do this, YOU HAD BETTER COMMENT ON WHY YOU COMMENTED IT OUT. Too often I come across a block of code that is just commented out. A quick glance means it had a purpose, but for what reason, I'm not sure because apparently it isn't needed anymore. So then I ask "why?" But there is no answer to this question. This is possibly the worst feeling of working with code--the idea that something was there, now is not, but they left it in for some reason and you don't know why.

          Now you're left with a couple scenarios:

          • The code caused a bug, but had some sort of purpose.
          • The code was replaced, and the new implementation is experimental which leads to
            • Exactly which lines of code did it replace?
          • It is dead code

          It is like walking into an offline factory line. Most bystanders will simply think "I had better not touch this or it could cause serious problems" so the only people in full control was the last shift of factory workers for that line. Meanwhile similar lines are still operating so you're not exactly sure why this particular production line was halted.

          Meanwhile if the code was in version control, I get a much clearer picture from diff on what exactly changed. If you just comment out a giant block and leave it there, check it in, then diff shows me that 2 lines had a comment added (in the case of block comments) or all the lines are commented out (modified, for single line comments). Now I have to diff again and skip to the version before that to get the picture I want, but now I may see other changes that may not be relevant.

          Either way, if you are commenting out code for the first two scenarios I listed, you need to think again about your version control system and manage it better. Experimental or untested stuff should be branched, not visible to other devs otherwise people make mistake it for production code. If I see commented out code blocks, I'm simply going to delete them myself because I can't read your mind and never will be able to.

        • Re:The more..... (Score:5, Insightful)

          by Anonymous Coward on Monday January 07, 2013 @06:47AM (#42503111)

          I comment the code and leave it there for at least one minor version. If things are all right, I'll remove them. The code is in source control anyway.

          That's a fine sentiment for a one-man dev team with a relatively small codebase, where you know what you're doing and can keep a handle on things, but for larger teams, people who leave commented-out code in place are annoying for everyone else.

          As you said already, it's in source control anyway, so why bother leaving it there even for one version?

          The counter arguments are that it causes additional diffs and commits that are unnecessary, and it gets in the way for other developers -- it harms readability.

          And very often, despite best intentions, it ends up being left there for a lot longer than was needed. The original developer may not come back to that file for some time, and after just a few revisions, no-one is quite sure which bits of commented out code should be deleted and which should be left in place. Or maybe even maintained -- after all, if it's been left in place, that implies it might have to be re-instated, so if the code around it changes, the commented out code really ought to also be tweaked to keep it in line.

          No-one has time to stop and figure it out either; the temptation is always just to leave it well alone -- if it's been left there, then there must have been a reason, but it's not my problem, so I won't touch it. End result, a year or two down the line, and we have huge quantities of dead code floating around, which probably wouldn't work anyway even if it was re-instated, and someone has to waste time with an excersise to go through the codebase and remove it all.

          In your world of your own little projects, keeping dead code floating around may sound like a good idea, but in the real world it really isn't.

          • Definitely agree with this. Commented out code can be a severe aggravation, and not just for source control - searches can also bring up false positives. Better to clean it out entirely at the earliest opportunity.
    • by paulatz (744216)

      The more you can delete, the better.

      Starting from the Murphy's law on programming:
      Every non trivial program has at least one bug

      You can derive by rigorous analogy the Murphy's law on not-programming:
      Every non written code has exactly zero bugs

      • by Chrisq (894406) on Monday January 07, 2013 @05:18AM (#42502653)

        The more you can delete, the better.

        Starting from the Murphy's law on programming: Every non trivial program has at least one bug

        You can derive by rigorous analogy the Murphy's law on not-programming: Every non written code has exactly zero bugs

        I have it - the holy grail, the key to bug free coding. If the deleted code has no bugs just restore the deleted lines and delete the rest. You will then have bug free programs.

      • by dkleinsc (563838)

        Not the first example - Ken Thompson predates that with his "One of my most productive days was throwing away 1000 lines of code."

    • Re:The more..... (Score:5, Insightful)

      by Marxdot (2699183) on Monday January 07, 2013 @04:28AM (#42502391)

      "It seems that perfection is reached not when there is nothing left to add, but when there is nothing left to take away" --Antoine de Saint Exupéry

      That is the only rule of deleting code; if you can take it away or do it more simply without causing Bad Things, do it.

  • by symbolset (646467) * on Monday January 07, 2013 @03:07AM (#42501991) Journal
    Delete away! It's not like legacy versions aren't available. Most code should be deleted. Almost all production code is bad.
    • by myurr (468709) on Monday January 07, 2013 @03:29AM (#42502089)

      Revision control is just one aspect. If you aren't the only person working on the code, or if there are other external dependencies such as you publish an API that others depend upon, then some form of automated testing will help ensure you haven't broken anything by deleting that code. Heck even if you are the sole developer then automated tests are still an incredibly good thingl.

      If you are deleting code within a larger project then try deprecating the code first. Flag it as deprecated in the documentation (you do have documentation don't you?) and put log messages in the code that warn whenever it is called. After a while you can be confident that it is unused (or only used in obscure rarely called functions) and you can delete with confidence.

      Revision control is your safety net, your last line of defence if you erroneously delete code that is in fact needed. It also allows you to always refer back to that code. Good commit messages will also help.

      • Most IDEs allow you to select the method/function with that code and display a call graph. So log messages are the most useless idea ...

        • by LordLucless (582312) on Monday January 07, 2013 @03:57AM (#42502235)

          Let me know how well that goes for you when your IDE tries to generate a call graph including third party applications that interface with yours via an HTTP API.

          • That goes superb.

            There is no difference wether my function is called by another low level function or by an HTTP endpoint, or do you believe an external system can call my function "just so"?

        • That's a useless assumption when dealing with dynamically loaded/dispatched code, or editing library code that will be pulled in by multiple projects. Logging always shows what's actually in use during a run.

          Besides, if you break the static call graph, the compiler will complain anyway. No need to graph.

          • or editing library code that will be pulled in by multiple projects.
            When you can not make sure the code is not used anymore then imho there is no reason to delete it.

            Besides, if you break the static call graph, the compiler will complain anyway. No need to graph.
            What is that supposed to mean? The compiler complains because a method/function is no longer called? That is new to me.

          • by uncqual (836337)
            The problem usually isn't that the entire procedure is unused (that's fairly easy to catch). Usually it's that some chunk of the procedure "appears" to be unreachable because callers should no longer (or perhaps ever should have) invoke(d) the procedure with parameters and state that causes the code in question to be executed.
  • what best practices do /.'s use when they modify production code?"

    Should be able to easily revertable

    • Re:Revertable (Score:5, Interesting)

      by gagol (583737) on Monday January 07, 2013 @03:13AM (#42502027)
      When in doubt, comment out and document. I also like to keep a commented (disabled by comments) generic version of a function if I have to work a heavily optimised version. For future generations, and keep sanity when code is revisited.
      • I try to keep an uncommented version.

        In debug mode, the optimized one will call the simple one and check the results are equal.

        Doesn't work easily in all cases if the function erforms mutation though.

  • by Marillion (33728) <ericbardes AT gmail DOT com> on Monday January 07, 2013 @03:08AM (#42502003)
    I really like git, but the key thing is to keep revision history. Deleted code is then never "deleted" it's just no longer cluttering up the screen. Of course it does mean you need to actually learn how to use a version control system beyond blind forward checkins.
    • by Anonymous Coward on Monday January 07, 2013 @03:59AM (#42502253)

      Version control is not your friend when you use it like this. It becomes your worst fucking enemy.

      Unless somehow your version control system somehow magically achieves the ability to imbue the knowledge of all the things that you tried and failed, and why they failed.

      If you don't want the older, now obsolete and/or broken code to be seen.... get a proper IDE with code folding. Using the VCS for this is definitely using the wrong tool for the job.

      If you try something obvious, and it doesn't work, odds are very good that the next bozo to come along is going to try exactly the same thing, and the odds are that they're not going to test it as well as you did.*

      You need to comment the code out, and you need to add a short explanation of why it doesn't work. E.g. // this looks obvious, but because of foo baz and kwok should never be attempted. YourName Date

      Then, when the idiot next in line tries to make exactly the change you said wouldn't work, (because you know that they will), and you are told to figure out why they broke the build, when you do figure out why it's broken, you can go back in the archive, pull out your previous attempt and explanation, and then clearly demonstrate that it isn't your fault.

      Version control is an archiving and accountability tool. Not something to magically hide the code until you need it, because if it that's how you use it, you'll never know that the previous version was there. Thus it fails at the most important thing code must do - communicating clearly to the programmers. And communicating what doesn't work (and why) is well over half the battle, more like 80 or 90 percent. (Yet another example of the Pareto Principle in action)

      TLDR: deleting old code to hide it epic fails the DRY principle

      • by mwvdlee (775178) on Monday January 07, 2013 @04:26AM (#42502379) Homepage

        deleting old code to hide it epic fails the DRY principle

        The purpose was not to hide code, but to delete code.
        If code is no longer valid and maintained, it shouldn't still be available as if it were.

      • Re: (Score:3, Insightful)

        by Anonymous Coward

        Unless somehow your version control system somehow magically achieves the ability to imbue the knowledge of all the things that you tried and failed, and why they failed.

        Thats why you
        a) Comment well in your code explaining the algorithm being used, which gets preserved in VCS alongside the code
        b) Correctly document adding/removing functionality in the commit log

        If you don't want the older, now obsolete and/or broken code to be seen.... get a proper IDE with code folding. Using the VCS for this is definitely using the wrong tool for the job.

        This is about code removal (eg when refactoring, tuning etc). Not hiding.

        TLDR: deleting old code to hide it epic fails the DRY principle

        Quite the opposite generally. Usually when I'm removing code it's when I'm refactoring to generalise code precisely so it can be reused in multiple places. Keeping the previous code in place commented out would mean I was then repeating myself.

        W

  • Source control (Score:5, Informative)

    by foniksonik (573572) on Monday January 07, 2013 @03:12AM (#42502021) Homepage Journal

    Who cares? You've got source control (SC) right? And you write unit tests right? If so then new code will pass the tests. If you write some benchmarks on performance then you'll know that too.

    Build early, build often, build against test coverage and you've got Continuous Integration (CI). If you've got CI and SC then anyone can write new code and it will either pass the tests or it will break the build. If it breaks the build use SC to pull that crap out.

    Done and done.

  • Vigil (Score:4, Funny)

    by Anonymous Coward on Monday January 07, 2013 @03:17AM (#42502045)

    Let Vigil delete your code for you when it's wrong and must be punished.

    • Re:Vigil (Score:5, Informative)

      by rwaldin (318317) on Monday January 07, 2013 @03:59AM (#42502247)

      Ah, Vigil! What a wonderfully amusing language...

      https://github.com/munificent/vigil [github.com]

      But isn't a language that deletes code crazy?

      No, wanting to keep code that demonstrably has bugs according to its own specifications is crazy. What good could it possibly serve? It is corrupted and must be cleansed from your codebase.

      Vigil will do this for you automatically.

      Vigil deleted a function. Won't that cause the functions that call it to fail?

      It would seem that those functions appear to be corrupted as well. Run Vigil again and it will take care of that for you. Several invocations may be required to fully excise all bugs from your code.

      • by mwvdlee (775178)

        Vigil deletes code that does not pass assertions (or "swear", as they call it).
        What mechanism prevents those assertions from being wrong?

        • What mechanism prevents those assertions from being wrong?

          Drinking sufficeint alcohol before pressing [Enter] is the standard mechanism. (It has been known to fail, but the results are not normally painful at the time).

  • by Darinbob (1142669) on Monday January 07, 2013 @03:19AM (#42502053)

    It's often good to just delete. Sometimes however I feel the need to put in a comment reminding the reader that the old functionality is gone. For example, it could say "no need to support device X here" or "input has already been validated". That's because it's not always evident to look in source code history to notice that there used to be something there.

  • Don't necessarily agree with this, but we comment out all older code when diffs aren't used. Commented code blocks with the date and editor's name works for small projects on a very tight deadline and this makes things easier if we want to rollback. Bigger projects do use diffs.

    As always, this is going to be a case of "works best for my situation", rather than just "best" method to do xyz.

    From the article:

    3.Is the manager/tech lead aware of the problem, and if not, why not?

    Since I work on contract, I don'

    • This. I don't worry about unwritten rules, I follow the written rules.
    • by Jorgensen (313325)

      Please... Don't comment it out. If people want to see what it was like before, that's what version control is for. If you delete code, you should actually delete it.

      You should give the same courtesy to others as you expect from them: When reading code, you're interested in what it does, and how it does it. Not a history lesson.

  • Sigh (Score:2, Informative)

    by styrotech (136124)

    A link to a discussion on another site that was itself a link to a discussion on another site.

    I know it's easy stories, but really? Are the slashdot trolls really going to offer any unique and useful insights that Ars and Stack Overflow haven't already covered?

  • #1 Make sure your code is better.
    #2 If it isn't ... Don't get caught!

  • by WinstonWolfIT (1550079) on Monday January 07, 2013 @03:25AM (#42502075)

    Unused code gets deleted no exceptions. If there's no client demonstrating its value, its value is zero. The reasons are obvious in systems that scale into the hundreds of thousands to millions of lines of code.The way I describe it to my teams is, Just-In-Time development rather than Just-In-Case. You can have a conceptual framework in place and fill it in on demand without committing yourself to an orthogonal matrix of features that 'might be useful'.

  • by EuclideanSilence (1968630) on Monday January 07, 2013 @03:36AM (#42502123)

    Ok so you are working on a team and deleting code. Here are some basic rules to follow.
    1) Don't delete your boss's code. Just change all the function calls to go around it. If he asks you about it, say that someone else changed it.
    2) Don't cuss out the guy who's code you are deleting until after you are done. You might have to ask him why he did certain things (unexpected library behavior etc).
    3) Make sure the code you add to replace the old code is longer than what you deleted. That way, you can tell your boss that you added 'x' lines of code to the project.
    4) Don't waste time unit testing your new code. Obviously if you have to replace the old code, then you are a better programmer than the last one. If the last programmer's code passed unit tests, and you are better, then obviously yours would pass unit tests too.
    Any politeness the other programmer shows to you after you delete his code is not to be trusted. The code we write is pride to us, and deleting someone else's code is like seeing your coworkers girlfriend naked in the shower. Sure, it's not really your fault and you didn't really do anything bad. But expect some negative passive aggressive behavior in response.

  • Check your test suite covers all the functionality you want your program to have. If you're feeling paranoid, create a couple of tests that WILL fail when the undesirable code is deleted. Make sure everything is in the repo. Maybe branch/tag. Delete, repeat tests, roll-back or checkin (after deleting the canary tests added above) and move on to the next thrilling episode in your coding career.

    The first sentence of the paragraph above is of cours the killer - without a full-stack suite of tests there'll alwa

  • Many places have standards for deletion of code, this is of course assuming by deletion you mean completed gone not just archived in source control. Deleting old code is like deleting history, if it is not there to learn from you are destined to eventually repeat the same mistakes. We never delete code that has been live and in production, comment it out or archive it off in source control, nothing that made it through to a live system is ever under any circumstances deleted.
  • by inglorion_on_the_net (1965514) on Monday January 07, 2013 @03:46AM (#42502177) Homepage

    There used to be written rules for deleting code. Then someone deleted them. And since we don't use version control, we have no way to get them back.

  • Delete whatever you want behind a well-defined API barrier, as long as it still does the job when you're done.

    Delete entire unused and obsolete APIs where appropriate.

    Individual function bodies count as APIs, as well as large modules.

  • Either you use VCS, and you can' t ever delete code because you use it properly. Then the answer is: You can delete whatever you like in the latest revision. If you don't use VCS you have problems that exceed the scope of deleted unused code by a factor 1000, and you shouldn't be allowed access to ANY production code.

  • by ElRabbit (2624627) on Monday January 07, 2013 @04:32AM (#42502417)
    ... for clarity
  • ...on these unwritten rules, they'd have been written down a long time ago.
  • by rollingcalf (605357) on Monday January 07, 2013 @05:01AM (#42502579)

    At a previous employer, there was a *written* rule for deleting significant blocks of code.

    If a properly functioning block of code already in production was being deleted due to changes in business requirements, a comment should be inserted at or near the point of deletion, which mentions that some code was deleted and the original code can be found in version x.xx, and preferably a phrase saying what it did.

    However, if the code was not yet released to the production system, or was being deleted because it's buggy, it was acceptable to simply delete it without leaving a comment (if somebody needed to research an old bug they could look in the bug tracker, which would show which version of the code last had the bug, so there was no need to mention the bug-deletion in the code).

  • While RCS may be well and good, it won't necessarily show what was deleted or why.

    My rule of thumb is to comment out the old stuff, and date it. If it is still there after a few months and the new stuff has been working without anyone noticing or commenting (in a negative fashion), this it's pretty safe to delete it and leave any history/archiving to RCS. Before doing the final delete, I will usually still put a comment in the function/proc header (if I hadn't already done so) about the deletion/major-chang

  • If you have a version control system in place. it's not an issue cause you can roll back or at least see the previous versions.

    If you're operating on a more basic level, clone the crufty code, put the cruft in comments explaining CLEARLY why you cut it out.

    When you fix or replace what you cloned, comment there as to exactly what you think was wrong or broken, and how you fixed it.

    One thing that is certain, with production code, you will NOT be the last person to work on it. Pay it forward with good prac

  • Personally I use the "Back Space" key but I have heard of strange perverted creatures who use the "Del" key :)
  • Delete it. Plain and simple.
    Put a comment/bug id in the commit log.
    What's the problem?

  • Rewriting code?
    version control + start from a working situation and end with a working situation. Where working situation means bugfree and no broken parts other than those that are going to be rewritten.

    Deleting code?
    version control.

  • by MobyDisk (75490) on Monday January 07, 2013 @11:22AM (#42505037) Homepage

    When removing code, consider commenting why it was removed. I've seen cases where code was removed then added back in causing bugs to reappear.

    Before:
    Log(“Shutting down”)
    CloseLog();
    Reboot();

    After:
    Log("Shutting down”)
    Reboot();

    Better:
    Log(“Shutting down”)
    // Don’t close the log file here because some other thread might still
    // write to it during shutdown/reboot, which could cause an error dialog
    // that prevents shutdown. The file will be closed anyway by the OS.
    Reboot();

"When it comes to humility, I'm the greatest." -- Bullwinkle Moose

Working...