5 ms·

> Code in comments [...] When asked applicants are usually well aware that commented-out-code is confusing, but somehow they almost always have it in their code

by vog 12y ago

> Code in comments [...] When asked applicants are usually well aware that commented-out-code is confusing, but somehow they almost always have it in their code. To my experience this is a common symptom of not using a version control system. You change a line of code, but you want to be able to undo if it doesn't work, possiblay half an hour later when you changed other places of your code (so your editor's undo is of no use). I also see this from people who are using VCS, but that's mostly because they are overcautious and/or have not really gotten "warm" with the VCS. A final, very small percentage are familar with VCS and confident with their code, but leave that commented stuff in there accidentally. The usual cause it that they have not (yet) acquired the habit of reviewing the diff before committing.

irremediable 12y ago
Yeah... I need to stop doing this. Hangover from pre-VCS academic code-soup habits.
danieltillett 12y ago
I am guilty of commenting out code when I am working on a problem, but before committing I remove it unless it is the simple version of some function that I made way too complex in order to improve performance. I find it easier to understand later what the complex code is supposed to be doing if I have the simple implementation in the comments.
- wmt 12y ago
  I wouldn't call the simpler implementation "commented out code", as it's doing what the best comments are doing, making complex code more readable. Having random scribbles, alt solutions and other "shitty" comments while working on a problem is definitely not a sin if you clean them up before pushing them forward.
- jerven 12y ago
  I suggest you move the simple version into a unit test case. Making sure the simple version and the complicated one have the same results. Plus I like to add some perf test that shows the complicated version is faster than the simple one that I can rerun after major upgrades to the lower level system e.g. when upgrading from java 1.5 to 1.6 I could remove some complicated code because the simple one was then as fast due to an improved JIT.
  - danieltillett 12y ago
    Yes I also think this is what I should do to, unfortunately when I first wrote the code I spend most of my time working on I didn't write unit tests at the function level (all my tests are at the module level). I have been meaning to get around correcting this, but the task is now so huge I fear it would take me more than a year to write all the unit tests :(
    - jerven 12y ago
      IMHO every automated test helps. Don't aim for 100% test coverage aim at using unit tests to develop faster and more efficiently and the test coverage stats will come automatically. I personally need to improve my TDD, but I am quite happy with my test driven bug fixing approach. When working on a bug I first turn it into a JUnit or selenium test, and then fix the bug and make the test pass.
      - deleted 12y ago
        [deleted]
- UrMomReadsHN 12y ago
  No... you are making your code hard to understand to others by doing that. Thats if you check it in like that. Nothing wrong with leaving it there during development.
  - gknoy 12y ago
    I disagree. If I see 25-40 lines of something that LOOKS like it should be simple, it's tempting to replace it with the four line version. A comment which says, "You might think this would be better as {{4 lines}}, but it is too slow... because this/that/etc" can be VERY helpful, so that one doesn't reproduce the problems (and the time fixing) that caused the initial un-simplification in the first place.
    - UrMomReadsHN 12y ago
      Thats totally different than just leaving it there without any context at all. Which is what i was referring to. An actual comment is of course helpful, but when people check in code that has been commented out it usually has no explanation at all. Thats the confusion part. I'm talking stuff like this...(and I believe so was the author)... //int foo = 10; int foo = 20; if (bar) //if (foo > 0) { foo++; } //else //{ //foo--; bar = false; //}
    - danieltillett 12y ago
      I do explain in the comments why the simple function was removed and why the complex version is being used. The biggest problem I have found over time is divergence of the simple version from the complex version. I try and avoid this by updating and testing both versions, but whenever you have code duplication there is a risk that they will diverge.
TeMPOraL 12y ago
> I also see this from people who are using VCS, but that's mostly because they are overcautious and/or have not really gotten "warm" with the VCS. Yeah, that could probably describe me. Reverting a particular piece of code back from VCS seems like more work (the kind of "I need to think about it" work), which is enough for me to leave bits of code commented when working on some particular task. I never leave them there for long - usually only until I can get the new piece of code working and tested. The commented code serves as a reference/fallback.
- k-mcgrady 12y ago
  This is what I do as well. I use VCS but it's more work. If I'm just refactoring a piece of code I comment it out while I work on the new stuff and test it. It also has the added benefit that I can reference the old code easily if needs be. Occasionally I will commit with the commented code still in place (until I'm 100% confident with my new code) but I usually follow that up quite quickly with a commit removing it.
  - mod 12y ago
    I'm roughly in this boat as well, though I don't want to be. I'd rather be intimate with git.
UrMomReadsHN 12y ago
We use version control yet there are people I work with that constantly check in commented out code. Even when asked to stop. Its sloppiness and not paying attention to details or caring.
eric_h 12y ago
We have a saying over here - TWGIF[1]. Any time someone checks in commented out code, they get chastised. 1: TWGIF - That's what git is for
- loqi 12y ago
  I've gotten in the habit of adding trailing whitespace when I comment out code or do any printf debugging. It's still quick, but then git yells at me if I try and commit it.
  - eric_h 12y ago
    That's an interesting way of dealing with it - I have my text editor strip trailing whitespace and ensure there's a newline at the end of the file on every save, so it wouldn't work for me. That's okay though. I visually review the diff before I commit (almost) every time.
gabemart 12y ago
> I also see this from people who are using VCS, but that's mostly because they are overcautious and/or have not really gotten "warm" with the VCS. I think that's partially because many developers will "learn" a VCS when working on a school project or hobby project by themselves. For a personal project, often all you really need is dead-simple versioning, and even then you may complete an entire project with just a linear series of commits on a single branch. It's easy to avoid really learning a VCS in that environment.
jolt 12y ago
Another reason for keeping the commented-out-code, could be if reverting to a previous version of the code is more work, than checking in a new version. Or when the old commented-out-code is used as an explanation as to why the current code looks as it does.
jimmaswell 12y ago
Maybe for just a small code segment, a comment is a lot more convenient than going all the way through git for it? What if there's some functionality in a method you're not sure you want to take out or change or not, but you've also added to the rest of the file, so you commit it with a small code segment commented out and a short explanation? That's not hurting anything and it'd be a PITA and overkill to put it in a different branch or make a separate commit only including it or something.
- npsimons 12y ago
  Maybe for just a small code segment, a comment is a lot more convenient than going all the way through git for it? I don't get this; stashing changes, branching, even just copying and pasting something from history in git, are all very fast for me (using git in emacs with egg). Granted, I can comment-dwim with M-;, and I've conditioned myself against "just" commenting out code, but still.
dllthomas 12y ago
Code should exist in comments when it's going to be uncommented shortly. This, because 1) reaching into the VCS is extra work compared to uncommenting something already present (think cache vs disk), and 2) more importantly, it is code that anyone making changes nearby should be aware of. Code in comments may also occur as a symptom of poor VCS habits, but that's not always the case.

by vog 12y ago

irremediable 12y ago

Yeah... I need to stop doing this. Hangover from pre-VCS academic code-soup habits.

danieltillett 12y ago

I am guilty of commenting out code when I am working on a problem, but before committing I remove it unless it is the simple version of some function that I made way too complex in order to improve performance. I find it easier to understand later what the complex code is supposed to be doing if I have the simple implementation in the comments.

wmt 12y ago
I wouldn't call the simpler implementation "commented out code", as it's doing what the best comments are doing, making complex code more readable. Having random scribbles, alt solutions and other "shitty" comments while working on a problem is definitely not a sin if you clean them up before pushing them forward.
jerven 12y ago
I suggest you move the simple version into a unit test case. Making sure the simple version and the complicated one have the same results. Plus I like to add some perf test that shows the complicated version is faster than the simple one that I can rerun after major upgrades to the lower level system e.g. when upgrading from java 1.5 to 1.6 I could remove some complicated code because the simple one was then as fast due to an improved JIT.
- danieltillett 12y ago
  Yes I also think this is what I should do to, unfortunately when I first wrote the code I spend most of my time working on I didn't write unit tests at the function level (all my tests are at the module level). I have been meaning to get around correcting this, but the task is now so huge I fear it would take me more than a year to write all the unit tests :(
  - jerven 12y ago
    IMHO every automated test helps. Don't aim for 100% test coverage aim at using unit tests to develop faster and more efficiently and the test coverage stats will come automatically. I personally need to improve my TDD, but I am quite happy with my test driven bug fixing approach. When working on a bug I first turn it into a JUnit or selenium test, and then fix the bug and make the test pass.
    - deleted 12y ago
      [deleted]
UrMomReadsHN 12y ago
No... you are making your code hard to understand to others by doing that. Thats if you check it in like that. Nothing wrong with leaving it there during development.
- gknoy 12y ago
  I disagree. If I see 25-40 lines of something that LOOKS like it should be simple, it's tempting to replace it with the four line version. A comment which says, "You might think this would be better as {{4 lines}}, but it is too slow... because this/that/etc" can be VERY helpful, so that one doesn't reproduce the problems (and the time fixing) that caused the initial un-simplification in the first place.
  - UrMomReadsHN 12y ago
    Thats totally different than just leaving it there without any context at all. Which is what i was referring to. An actual comment is of course helpful, but when people check in code that has been commented out it usually has no explanation at all. Thats the confusion part. I'm talking stuff like this...(and I believe so was the author)... //int foo = 10; int foo = 20; if (bar) //if (foo > 0) { foo++; } //else //{ //foo--; bar = false; //}
  - danieltillett 12y ago
    I do explain in the comments why the simple function was removed and why the complex version is being used. The biggest problem I have found over time is divergence of the simple version from the complex version. I try and avoid this by updating and testing both versions, but whenever you have code duplication there is a risk that they will diverge.

TeMPOraL 12y ago

> I also see this from people who are using VCS, but that's mostly because they are overcautious and/or have not really gotten "warm" with the VCS. Yeah, that could probably describe me. Reverting a particular piece of code back from VCS seems like more work (the kind of "I need to think about it" work), which is enough for me to leave bits of code commented when working on some particular task. I never leave them there for long - usually only until I can get the new piece of code working and tested. The commented code serves as a reference/fallback.

k-mcgrady 12y ago
This is what I do as well. I use VCS but it's more work. If I'm just refactoring a piece of code I comment it out while I work on the new stuff and test it. It also has the added benefit that I can reference the old code easily if needs be. Occasionally I will commit with the commented code still in place (until I'm 100% confident with my new code) but I usually follow that up quite quickly with a commit removing it.
- mod 12y ago
  I'm roughly in this boat as well, though I don't want to be. I'd rather be intimate with git.

UrMomReadsHN 12y ago

We use version control yet there are people I work with that constantly check in commented out code. Even when asked to stop. Its sloppiness and not paying attention to details or caring.

eric_h 12y ago

We have a saying over here - TWGIF[1]. Any time someone checks in commented out code, they get chastised. 1: TWGIF - That's what git is for

loqi 12y ago
I've gotten in the habit of adding trailing whitespace when I comment out code or do any printf debugging. It's still quick, but then git yells at me if I try and commit it.
- eric_h 12y ago
  That's an interesting way of dealing with it - I have my text editor strip trailing whitespace and ensure there's a newline at the end of the file on every save, so it wouldn't work for me. That's okay though. I visually review the diff before I commit (almost) every time.

gabemart 12y ago

> I also see this from people who are using VCS, but that's mostly because they are overcautious and/or have not really gotten "warm" with the VCS. I think that's partially because many developers will "learn" a VCS when working on a school project or hobby project by themselves. For a personal project, often all you really need is dead-simple versioning, and even then you may complete an entire project with just a linear series of commits on a single branch. It's easy to avoid really learning a VCS in that environment.

jolt 12y ago

Another reason for keeping the commented-out-code, could be if reverting to a previous version of the code is more work, than checking in a new version. Or when the old commented-out-code is used as an explanation as to why the current code looks as it does.

jimmaswell 12y ago

Maybe for just a small code segment, a comment is a lot more convenient than going all the way through git for it? What if there's some functionality in a method you're not sure you want to take out or change or not, but you've also added to the rest of the file, so you commit it with a small code segment commented out and a short explanation? That's not hurting anything and it'd be a PITA and overkill to put it in a different branch or make a separate commit only including it or something.

npsimons 12y ago
Maybe for just a small code segment, a comment is a lot more convenient than going all the way through git for it? I don't get this; stashing changes, branching, even just copying and pasting something from history in git, are all very fast for me (using git in emacs with egg). Granted, I can comment-dwim with M-;, and I've conditioned myself against "just" commenting out code, but still.

dllthomas 12y ago

Code should exist in comments when it's going to be uncommented shortly. This, because 1) reaching into the VCS is extra work compared to uncommenting something already present (think cache vs disk), and 2) more importantly, it is code that anyone making changes nearby should be aware of. Code in comments may also occur as a symptom of poor VCS habits, but that's not always the case.