[giberson]

>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

[ajross]

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]

> 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?

[lamontcg]

And solving the actual problem would be much easier with test cases than an algorithm. This is made ever more important if the original implementation is "complex and likely to have subtle bugs" since that implies that anyone reading the comment is likely to miss 'seeing' important edge cases.

A really simple way to solve this would be to move the simplified algorithm to a test suite which iterated over some important edge conditions and validated that the simplified algorithm and the refactored code agreed. That would preserve the simplified code, but in a much more useful form where automated testing could be done against it.

[draw_down]

> That's the kind of smug naivete that makes young programmers so insufferable.

You said it.

[Johnny555]

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/

[pyre]

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.

[bastawhiz]

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.

[eximius]

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]

Good thing it was a toy.

[keyle]

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.

[reddytowns]

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]

Where is <here> and will it always be there?