6 ms·

>we leave this version here commented-out, because the code is very complex and likely to have subtle bugs. If bugs _are_ found, it might be of interest

>we leave this version here commented-out, because the code is very complex and likely to have subtle bugs. If bugs _are_ found, it might be of interest to look at the old code and see what did it do in the relevant situation. Here are two better alternative solutions: * comments (in natural language not code) that describe what the code should do * test cases: to dictate what code should do and prove it does do it

bastawhiz 11y ago
It's a bit ironic that there's a longer comment describing why the code shouldn't be removed than there are comments describing what the code _actually does_ or _why_ inline.
pyre 11y ago
You missed: * Comments referencing the original code by date / version / commit id. Date and version are relevant because the comment might not get updated if there is ever another VCS migration.
Johnny555 11y ago
That might be workable idea if the guy writing the replacement code has such a perfect understanding of the original code that he can spell out every assumption and edge condition that the code covers (as well as write tests to cover all conditions). The best you can hope for is comments and tests that cover the new author's impression of what the old code was doing, which is not necessarily the same as what the old code did. In this case, since he apparently doesn't feel that he has that perfect understanding, he left the original code there, so when someone says "Hey! I counted on behavior XXX, this change broke my workflow!". And any change no matter how trivial or "correct" always breaks someone's workflow. https://xkcd.com/1172/ https://xkcd.com/1172/
ajross 11y ago
That's the kind of smug naivete that makes young programmers so insufferable. Your use of "should" in your two sentences is a hidden, inappropriate abstraction. You're positing the existence of a "correct design" for expand-file-name and working backwards to how you'd write it in a modern ground-up development project. The actual problem at hand was rewriting that function in a way that doesn't break the uncounted thousands of usages in the wild, which depend on the self-described "complex and likely to have subtle bugs" original implementation.
- daenz 11y ago
  > That's the kind of smug naivete that makes young programmers so insufferable. That is really toxic language. It's important to call out bad practices where they exist. Not having good comments and test cases for intended behavior is a bad practice. Yes, in the real world, plenty of code gets shipped without adhering to these practices, but it's really unfortunate to see someone talked down on for showing where good practices could have prevented the current circumstances. AFAIK, neither of you have personally written those lines of code, so why is ego getting in the way here?
  - eitally 11y ago
    Because with experience comes internalization of the maxim, "perfect is the enemy of good." Mature programmers get more done and ship more good code than they can remember, while junior programmers are so focused on following perfect standards using perfect frameworks and perfect syntax that they 1) ship very slowly, and 2) belabor every decision, causing even more bugs. Yes, the language isn't the most ... diplomatic ... but it is also frank & honestly expresses one difference a decade of experience makes for most professional programmers.
    - deleted 11y ago
      [deleted]
    - colund 11y ago
      > Mature programmers get more done and ship more good code than they can remember, while junior programmers Excellent insight!
  - guest1539 11y ago
    > so why is ego getting in the way here? I find those kinds of comments insufferable as well, but I'll assume you actually wanted an answer and not just an excuse to chastise someone: It's because those of us who have learned we don't know everything are annoyed by people who still act like they do know everything. It hurts because we're embarrassed by remembering our younger selves.
keyle 11y ago
Only comment if the code is doing something tricky, or unclear and has been written in such a way for a reason worth noting. Don't write comments about what the code should do. If you have to write something, comment on the domain/context, not the code.
eximius 11y ago
I wonder what the most complex algorithm you've ever implemented was. The most complex I ever implemented was a toy crypto system that I implemented first naively, then again with simple optimizations. In neither case was there a simple way to describe what the code should do because it was inherently complex.
- woah 11y ago
  Good thing it was a toy.
reddytowns 11y ago
How about: The old version of this code is under source-control <here>. The code is very complex and likely to have subtle bugs. If bugs _are_ found, it might be of interest to look at the old code and see what did it do in the relevant situation.
- gaius 11y ago
  Where is <here> and will it always be there?
  - V-2 11y ago
    What is "always" and do we have to care about always? Sounds like splitting hairs to me. Even the comment in question admits: "it's true that it will be accessible from the repository". Only then it goes on to say "but a few years from deletion, people will forget it is there" - well, then just leave a comment reminding them about it. I see no plausible reason for keeping dead, commented out code in the codebase when you've got VCS.

bastawhiz 11y ago

It's a bit ironic that there's a longer comment describing why the code shouldn't be removed than there are comments describing what the code _actually does_ or _why_ inline.

pyre 11y ago

You missed: * Comments referencing the original code by date / version / commit id. Date and version are relevant because the comment might not get updated if there is ever another VCS migration.

Johnny555 11y ago

That might be workable idea if the guy writing the replacement code has such a perfect understanding of the original code that he can spell out every assumption and edge condition that the code covers (as well as write tests to cover all conditions). The best you can hope for is comments and tests that cover the new author's impression of what the old code was doing, which is not necessarily the same as what the old code did. In this case, since he apparently doesn't feel that he has that perfect understanding, he left the original code there, so when someone says "Hey! I counted on behavior XXX, this change broke my workflow!". And any change no matter how trivial or "correct" always breaks someone's workflow. https://xkcd.com/1172/ https://xkcd.com/1172/

ajross 11y ago

That's the kind of smug naivete that makes young programmers so insufferable. Your use of "should" in your two sentences is a hidden, inappropriate abstraction. You're positing the existence of a "correct design" for expand-file-name and working backwards to how you'd write it in a modern ground-up development project. The actual problem at hand was rewriting that function in a way that doesn't break the uncounted thousands of usages in the wild, which depend on the self-described "complex and likely to have subtle bugs" original implementation.

daenz 11y ago
> That's the kind of smug naivete that makes young programmers so insufferable. That is really toxic language. It's important to call out bad practices where they exist. Not having good comments and test cases for intended behavior is a bad practice. Yes, in the real world, plenty of code gets shipped without adhering to these practices, but it's really unfortunate to see someone talked down on for showing where good practices could have prevented the current circumstances. AFAIK, neither of you have personally written those lines of code, so why is ego getting in the way here?
- eitally 11y ago
  Because with experience comes internalization of the maxim, "perfect is the enemy of good." Mature programmers get more done and ship more good code than they can remember, while junior programmers are so focused on following perfect standards using perfect frameworks and perfect syntax that they 1) ship very slowly, and 2) belabor every decision, causing even more bugs. Yes, the language isn't the most ... diplomatic ... but it is also frank & honestly expresses one difference a decade of experience makes for most professional programmers.
  - deleted 11y ago
    [deleted]
  - colund 11y ago
    > Mature programmers get more done and ship more good code than they can remember, while junior programmers Excellent insight!
- guest1539 11y ago
  > so why is ego getting in the way here? I find those kinds of comments insufferable as well, but I'll assume you actually wanted an answer and not just an excuse to chastise someone: It's because those of us who have learned we don't know everything are annoyed by people who still act like they do know everything. It hurts because we're embarrassed by remembering our younger selves.

keyle 11y ago

Only comment if the code is doing something tricky, or unclear and has been written in such a way for a reason worth noting. Don't write comments about what the code should do. If you have to write something, comment on the domain/context, not the code.

eximius 11y ago

I wonder what the most complex algorithm you've ever implemented was. The most complex I ever implemented was a toy crypto system that I implemented first naively, then again with simple optimizations. In neither case was there a simple way to describe what the code should do because it was inherently complex.

woah 11y ago
Good thing it was a toy.

reddytowns 11y ago

How about: The old version of this code is under source-control <here>. The code is very complex and likely to have subtle bugs. If bugs _are_ found, it might be of interest to look at the old code and see what did it do in the relevant situation.

gaius 11y ago
Where is <here> and will it always be there?
- V-2 11y ago
  What is "always" and do we have to care about always? Sounds like splitting hairs to me. Even the comment in question admits: "it's true that it will be accessible from the repository". Only then it goes on to say "but a few years from deletion, people will forget it is there" - well, then just leave a comment reminding them about it. I see no plausible reason for keeping dead, commented out code in the codebase when you've got VCS.