7 ms·

> Don't comment out code, just remove I hate commented out code, but this doesn't work in every situation. While working on a clinical information system, we h

by dt3ft 5y ago

> Don't comment out code, just remove I hate commented out code, but this doesn't work in every situation. While working on a clinical information system, we had frequent "please bring it back" requests that came a few months after things were removed (also by request). Relying on source control is nice, but unless you document every feature/removal or use very extensive commit messages, finding the code that was removed (especially if it was spread over several classes) is easier if you just comment it out and write why it was commented out. The important part here is to add why something was commented out.

karottenreibe 5y ago
In my experience you absolutely SHOULD document every request and removal in e.g. an issue tracker. It's very valuable information. And commented out code gets stale so quickly. Much better to check out an old version where it still worked than uncommenting a bunch of lines that now have compile errors and have become incomprehensible. Happens so easily, e.g. after some variable renames.
dtech 5y ago
Seems garbage advice to me. Now you've polluted your production code, likely forever. I don't see a situation where using an issue tracker + good commits - e.g. 1 commit for the 1 PR linked to the ticket that removes the feature - won't result in a superior situation from a maintenance and traceability perspective.
- serial_dev 5y ago
  Yes, I agree with you. Another thing is how do you know which of the removed features will be asked to be put back? You can't really know. You can guess, but that will leave you with many classes/functions commented out only in the hope that one day one of them will be requested to be put back. This leads to nobody daring to remove old code for months/years. Then, one day, a new coder on the team just ask WTF is this here, realize it's outdated and not running anymore, and will remove the whole thing. Then, the "senior" guy on the team comes shouting at him/her: "We might need it one day, what do you know about our team, bla bla bla, we had a feature 4 years ago that was removed and we had to add back, so now we have the rule of commenting things out". Yuck, what a nightmare. Just create good commit messages, don't group irrelevant changes together, and you'll be able to find the commit if you really have to. And even if you can't, you know you can still just develop that thing again, right?
  - zozbot234 5y ago
    I thought the best practice for "keeping stuff around just in case" was an attic/ directory in the source repo. That way you're still benefiting from version control, but it's also clear that the code is totally untested and might bitrot at any time.
    - dtech 5y ago
      There's little advantage over that vs looking up the old files in source control.
  - RMPR 5y ago
    One solution might be to create a separate branch with the feature still available. With time, you kind of get the gut feeling of what could be useful later and what could not.
- dt3ft 5y ago
  Sure, in a normal world. But with 2 devs in team, handling a clinical information system in very rapidly changing environment (new features), there is no time to document things in this way.
  - qayxc 5y ago
    Jump ship then. This sounds like you're travelling towards burnout at supersonic speed. Rapidly changing environment/features is just newspeak for customer has no idea what they even want in the first place and project management is non-existent.
    - dt3ft 5y ago
      I agree and did jump ship. This was in Switzerland, by the way.
  - dtech 5y ago
    OK, but that doesn't make it right or good advice. It's just one type of of technical debt due to too much workload.
cjfd 5y ago
Yes, sometimes (not often) commented out code can be defended. The point of adding why it is commented out is very well taken. Then you can remove it if the reason for it still being there is not longer applicable. And commented out code tends to go stale quite quickly so if it really is not needed anymore it should be removed. But I do wonder, if the better option would not have been to make the behavior that you commented out optional. Sometimes one can know beforehand whether a request is actually what people are going to want in the end. Once I wrote variant 1 of a certain feature. This was deemed not good and variant 2 was written. This was also not what people wanted so variant 3 was written. By then I got the inkling that also variant 3 would not be there to stay so I made it an option whether 2 or 3 was in effect. In the end, the final functionality was mostly a lot like variant 1 so the option turned out not be useful but maybe sometimes such optional behavior can sometimes be the best way....
cerved 5y ago
git log -p -- file
Siira 5y ago
I have started to prefix my actual comments with another character after the comment character, e.g., #: in Python. Then I know which comments are “real,” and which are just garbage code that can safely be ignored/deleted.
D-Coder 5y ago
> we had frequent "please bring it back" requests that came a few months after things were removed (also by request) Not exactly relevant to this discussion, but it sounds like a good case for a config flag. Let them turn it on and off as often as they like.

by dt3ft 5y ago

karottenreibe 5y ago

In my experience you absolutely SHOULD document every request and removal in e.g. an issue tracker. It's very valuable information. And commented out code gets stale so quickly. Much better to check out an old version where it still worked than uncommenting a bunch of lines that now have compile errors and have become incomprehensible. Happens so easily, e.g. after some variable renames.

dtech 5y ago

Seems garbage advice to me. Now you've polluted your production code, likely forever. I don't see a situation where using an issue tracker + good commits - e.g. 1 commit for the 1 PR linked to the ticket that removes the feature - won't result in a superior situation from a maintenance and traceability perspective.

serial_dev 5y ago
Yes, I agree with you. Another thing is how do you know which of the removed features will be asked to be put back? You can't really know. You can guess, but that will leave you with many classes/functions commented out only in the hope that one day one of them will be requested to be put back. This leads to nobody daring to remove old code for months/years. Then, one day, a new coder on the team just ask WTF is this here, realize it's outdated and not running anymore, and will remove the whole thing. Then, the "senior" guy on the team comes shouting at him/her: "We might need it one day, what do you know about our team, bla bla bla, we had a feature 4 years ago that was removed and we had to add back, so now we have the rule of commenting things out". Yuck, what a nightmare. Just create good commit messages, don't group irrelevant changes together, and you'll be able to find the commit if you really have to. And even if you can't, you know you can still just develop that thing again, right?
- zozbot234 5y ago
  I thought the best practice for "keeping stuff around just in case" was an attic/ directory in the source repo. That way you're still benefiting from version control, but it's also clear that the code is totally untested and might bitrot at any time.
  - dtech 5y ago
    There's little advantage over that vs looking up the old files in source control.
- RMPR 5y ago
  One solution might be to create a separate branch with the feature still available. With time, you kind of get the gut feeling of what could be useful later and what could not.
dt3ft 5y ago
Sure, in a normal world. But with 2 devs in team, handling a clinical information system in very rapidly changing environment (new features), there is no time to document things in this way.
- qayxc 5y ago
  Jump ship then. This sounds like you're travelling towards burnout at supersonic speed. Rapidly changing environment/features is just newspeak for customer has no idea what they even want in the first place and project management is non-existent.
  - dt3ft 5y ago
    I agree and did jump ship. This was in Switzerland, by the way.
- dtech 5y ago
  OK, but that doesn't make it right or good advice. It's just one type of of technical debt due to too much workload.

cjfd 5y ago

Yes, sometimes (not often) commented out code can be defended. The point of adding why it is commented out is very well taken. Then you can remove it if the reason for it still being there is not longer applicable. And commented out code tends to go stale quite quickly so if it really is not needed anymore it should be removed. But I do wonder, if the better option would not have been to make the behavior that you commented out optional. Sometimes one can know beforehand whether a request is actually what people are going to want in the end. Once I wrote variant 1 of a certain feature. This was deemed not good and variant 2 was written. This was also not what people wanted so variant 3 was written. By then I got the inkling that also variant 3 would not be there to stay so I made it an option whether 2 or 3 was in effect. In the end, the final functionality was mostly a lot like variant 1 so the option turned out not be useful but maybe sometimes such optional behavior can sometimes be the best way....

cerved 5y ago

git log -p -- file

Siira 5y ago

I have started to prefix my actual comments with another character after the comment character, e.g., #: in Python. Then I know which comments are “real,” and which are just garbage code that can safely be ignored/deleted.

D-Coder 5y ago

> we had frequent "please bring it back" requests that came a few months after things were removed (also by request) Not exactly relevant to this discussion, but it sounds like a good case for a config flag. Let them turn it on and off as often as they like.