top | item 10839146

Please do not delete this commented-out version

413 points| jordigh | 10 years ago |emacshorrors.com

203 comments

order
[+] stcredzero|10 years ago|reply
There are places in Smalltalk code where you see:

    false ifTrue: [
        "Some code to debug to help understand..."
    ].
In VisualWorks, you can double-click next to the 1st square bracket to select the code, then instantly execute it or debug it. The reason why it's not simply commented out, is that automated refactorings in the Smalltalk Refactoring Browser can then touch the commented-out code. (St RB used to be guaranteed to always execute strict refactorings and produce correct code.)

Often such code can even use values from the current scope, so a separate debug session can be spawned to see what it "would have been like" then dismissed with no consequences. It's stuff like that which made Smalltalk an uber-environment back in the day.

[+] matthavener|10 years ago|reply
In clojure, you see something similar at the bottom of files:

  (comment
    (def some-state (create-state))
    (some-fn [1 2 3])
    (do-something some-state)
  )
The compiler ignores the block, but in a REPL enabled editor, you can evaluate the forms to possibly view current state, or run functions that test behaviors or probe runtime state.
[+] Nelson69|10 years ago|reply
It is possible, or even likely, that a repository switch could lose that history on a project like emacs.

Guile now has an elisp interpreter and guile itself has JIT capabilities as well as a number of other features elisp hackers have grumbled about over the years. It's been mostly one man's work but it has spanned years to get guile-elisp to where it is. The discussions about migrating to it have been somewhat interesting to read. You have a remarkably solid, cross platform, well tested product and they're very very cautious to harm that, even with big speed and feature improvements on the table. It's one of the few good examples of "maybe you want to think twice about fixing something that isn't broken." Then when/if they do fix it, I bet you'll be able to build it with the old elisp for 20 years, just in case something isn't quite right...

[+] makecheck|10 years ago|reply
Generally I find all copies to be bloat.

Seeing the original code will not necessarily explain old behavior. When you make a copy, how much are you planning to copy? To debug ancient code that calls 5 functions, you might need access to the histories of those 5 functions; and the histories of any functions they call, and heck maybe you only saw correct behavior on a certain OS or with certain input (none of which you may have access to). If you don't have all those histories, the "correct old code" you are looking at may even lie to you, as it suggests it is calling functions you think you are familiar with but those functions have changed in some important way.

After enough time passes — and at the rate this industry moves, time is short — the only thing that really matters is whether or not the program currently does what its current users require. And if it doesn't, you define what behavior you need it to have and find a way to fix it.

[+] carussell|10 years ago|reply
A while back, I found the following amusing comment in the source to GNU tar:[1]

    /* Parse base-64 output produced only by tar test versions
       1.13.6 (1999-08-11) through 1.13.11 (1999-08-23).
       Support for this will be withdrawn in future releases.  */
I did some digging then and found at that it had been introduced in a commit that was (at that time) almost 10 years old, to the day.[2]

1. http://git.savannah.gnu.org/cgit/tar.git/tree/src/list.c?id=...

2. http://git.savannah.gnu.org/cgit/tar.git/commit/?id=e4e62484...

[+] Havoc|10 years ago|reply
:( Disappointed. I was expecting a story along the lines of the "Magic / More Magic button" story.

edit http://catb.org/jargon/html/magic-story.html

[+] chc4|10 years ago|reply
Same. I've heard some horror stories about programs failing to compile if you switch independent lines around or, even worst, remove some comments. Thought it would be one of those!
[+] giberson|10 years ago|reply
>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
[+] craigds|10 years ago|reply
This doesn't seem all that awful. If the old code is significantly more readable, then perhaps it serves as good documentation about the new code.

It is somewhat surprising that this would last 30 years though. In our huge codebase, barely any lines have survived 8 years. Even less so the comments.

[+] Walkman|10 years ago|reply
There are tens of thousands of lines commented out in the codebase at my company... When I say "We should delete all of them" they defend "it will be good sometime" LOL. That's why we have a 500k LOC codebase and developing a feature takes weeks and months instead of days. I'm not allowed to refactor...
[+] theophrastus|10 years ago|reply
As something of a computational geneticist, i really appreciate this comment. because there are exon sequences in DNA, which are expressed as mRNA and often translated to proteins -- let's say "compiled"; and intron sequences which are spliced away and currently unused. However introns are the stuff of past and future genes and controls, just like lines of code commented out. They are like boxes of stuff in your attic, currently not used, but very handy when conditions change.
[+] coliveira|10 years ago|reply
When I am in such environments what I always to is to add some little cleanup to features that I am adding. In a code review, it is always easy to justify that I am also removing these 5 lines that surround the code and are not needed anymore. It takes time, but it helps.
[+] mrfusion|10 years ago|reply
I think ides should have a really easy way to see old code. I'm thinking like arrow buttons next to each function and you can browse though older versions.

It's true in theory all your old code is stored in vcs but it's such a bear to actually go look at it.

[+] sergiotapia|10 years ago|reply
That source code looks HORRIBLE. Is it just because of the language or because the person who wrote had no other choice?

http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=488759...

It must have been done for a reason right, emacs is incredibly old in tech years and solid as a rock.

[+] mratzloff|10 years ago|reply
You may not be familiar with reading and writing C. String parsing in ANSI C in the 1980s looked like this. And it is completely readable and obvious to those who are well-versed in the language and practices of the time.

For example, those naming practices were pretty essential when working with 80-character monitors.

And all those ifdefs were required for dealing with DEC minicomputers and (later) microcomputers. A minicomputer, if you didn't know, was something that in terms of power and cost existed between a mainframe and microcomputer (i.e., a PC).

Breaking out the code into different functions by platform (segmented by ifdef statements) would create unnecessary code bloat unless the implementations differed significantly. And believe it or not, this was much more readable (with the technology of the time) than calling separate functions within a master function, requiring referring to each function separately. Of course, ifdefs for different platforms are still required, and are often used in a similar way today.

I guess what I'm trying to say here is that there were very practical and pragmatic reasons code used to look like this, and that things much more obscure and (to modern eyes) hard to read were written by very smart people who knew exactly what they were doing.

[+] pjc50|10 years ago|reply
String manipulation in C: not even once. It always ends up looking like this. This contains some niceties like `p[-1]` as well, which is perfectly valid but upsets people trying to check whether it's guaranteed to stay in the buffer.

Quite a lot of the #ifdef damage is to deal with VMS.

[+] jhallenworld|10 years ago|reply
Eh, the code is not that bad but it's weird that they don't scan from the right instead of the left to find 'xxx//yyy//zzz' (which means ignore 'xxx/' because you began a new path with '/yyy', but ignore '/yyy' also because you began a new path with '/zzz').

Here is the same code in JOE (which has no VMS support and which treats foo/../bar and bar as different files - the original emacs code was written before symlinks!). JOE has a variable length string library, so you see 'vsncpy' instead of the alloca / strcpy / strcat / make_string.

http://sourceforge.net/p/joe-editor/mercurial/ci/default/tre...

Edit: Holy Cow! The new version in emacs is much more involved: https://github.com/emacs-mirror/emacs/blob/master/src/fileio...

"For technical reasons, this function can return correct but non-intuitive results for the root directory; for instance, (expand-file-name ".." "/") returns "/.."... " :-)

[+] jheriko|10 years ago|reply
Actually this code /is/ horrible in many ways (single letter variable names instead of something descriptive?!?), as is 'the old style' in general - which is why those practices are uncommon or considered 'bad' today

it can still be solid as a rock and be needlessly difficult to read, written in a poor style. they aren't mutually exclusive things, despite what many may preach.

[+] ef4|10 years ago|reply
Pretty much all portable C from that era looks like that.
[+] noobermin|10 years ago|reply
FWIW, I've seen worse code in closed source, computational codes. Still, I always thought GNU were in a better category than the rest of us.
[+] sputnik27|10 years ago|reply
Oh, god, so awful. All those ifdefs..

Funny: Tried to find the comment commenting it out. I found it. #IF 0. Wow.

[+] kazinator|10 years ago|reply
Yes, if the function breaks, do we care what some code did in the same situation that was removed 30 years ago?

I mean, let's look at it like this: does the author of the removed code have a vote in what should happen, through the comment? If it's so important, why was it removed?

How do we even know that the new code did something wrong? We do not know that in relation to the old function; we know that in relation to some modern criterion. Maybe it completely bombs (irrelevant that the old code didn't bomb, or whether or not it did), or that returns some result someone doesn't like (without regard for what the old code did), or that violates some documentation.

We can decide today what the code should do, and fix it, or its specification or both.

[+] Tinned_Tuna|10 years ago|reply
I'm surprised that we're still using this as a bit of a hack in the future. In the project I'm working on, when we substantially optimise a section of code, we move the un-optimised, believed-good, version. It becomes an oracle against which the optimised version is tested on both manually and randomly generated test cases.

I'm not saying that the above solution should have been deployed at the time, but perhaps it could be moved in that direction now that we have these techniques and tools to hand.

Could such a solution work well here? Perhaps, it would be even more useful, as tests would be likely to fail if the optimisation was bad, rather than just assisting with debugging after the fact.

[+] sxates|10 years ago|reply
My question is - has anyone needed to refer back to the old code in the last 30 years? I can understand wanting to have the old code around for easy reference in case you need to debug the refactored version, but if no bugs have come up in the last 30 years (or even, say, the last 5), it seems pretty safe to say the new code is solid.
[+] cfcef|10 years ago|reply
Another question: is the old code even valid in any sense compared to the current code? Since the original was written before 1985, then 99.9% of the people running Emacs will be running it on an OS that did not exist in any form in 1985 (Linux, Mac OS X, Windows, Android, etc) and the rest will be running on OSes which have changed substantially or been replaced entirely since then (SunOS->Solaris).
[+] chris_wot|10 years ago|reply
If you think that's bad, try looking at the LibreOffice source code. I think I found a 15 year old bug the other day [1] [2], but I only noticed this when I was trawling through the history of the file "outdev3.hxx", which via cgit was a pain in its own right.

There have, in fact, been 5 source code repository switches in LibreOffice over the last 30 years. [3] It started with some sort of proprietary version from 1988 to 2000 so there is no history at all from this period (which I think is a great loss...), from 2000 to 2009 they used CVS, then from 2008 to 2009 they used SVN, then from 2009 to 2011 it was Mercurial, then from 2011 to 2014 it was back to SVN, and LibreOffice started using git from the very start.

One of the worst practices that the OpenOffice.org team had was to get a whole bunch of work, bundle it up into a single commit and then note that there were multiple fixes in the commit. This makes it nigh-on impossible to untangle what fixed what. They apparently branched in SVN, then took the branch and merged it, but then blatted the original branch so now this is all lost in history.

And if you've ever read some of the most crufty parts of LibreOffice (cough VCL cough) then you'd understand that actually reading good commits is really very important for moments you think "what on EARTH is that code doing?!?"...

1. https://bugs.documentfoundation.org/show_bug.cgi?id=96826

2. http://randomtechnicalstuff.blogspot.com.au/2016/01/possible...

3. https://archive.fosdem.org/2014/schedule/event/exploring_ope...

[+] gonyea|10 years ago|reply
If you think this is crazy, never ~ever~ look at what Mainframes are running.
[+] jdcskillet|10 years ago|reply
I was just thinking the same thing... We have lines of commented out code from 1988 (yes. that is a correct date) that people flat out refuse to delete. I am so glad I got out of that mess...
[+] stevebmark|10 years ago|reply
Committing commented out code means you don't know how to use your version control system. A comment with hash of the commit of the old code, or a command to show an older version of the file using your vcs system, is much better, and more team-friendly.
[+] codemac|10 years ago|reply
Committing commented out code means you're trying to communicate with comments in your code. English isn't the only language that's useful in comments, especially when trying to document something of computable logic.

If you have any comments in your code at all, you could make the same argument about "not knowing how to use your version control system" as everything could be in commit logs.

And please define "team-friendly", it is absurd to suggest that the git cli makes anything friendly.

[+] incepted|10 years ago|reply
> Committing commented out code means you don't know how to use your version control system.

Read the linked article more carefully.

It's an example of how, sometimes, it's better to keep comments in the code than relying on source control to preserve them.

These situations are rare, but they happen.

People who say "Commented code should always be deleted, period" need to be a bit more open minded about the constraints in which we operate in the real world.

[+] lqdc13|10 years ago|reply
Sure but then you switch Version Control systems later and the hash/vcs command may not mean anything.
[+] 0x0|10 years ago|reply
Respectfully disagree. If it's not "in your face", you'll forget it's there - or even never have known about it, if you weren't on the project since the beginning. Also, it won't show up in simple greps. Not saying you should leave every old method around, but for some routines (or part of routines) it can be very instructive to surface what DIDN'T work.
[+] logicallee|10 years ago|reply
And including ANY comment in code - as in, mixing code and comments in the same file - means that your workflow doesn't support external documentation keyed to branches, hashes, filenames, and line numbers.

Likewise, naming variables, functions, classes, and continuously typing out the same name over and over again means that your language and IDE is not rich enough to contain pronouns, making you have to type tediously instead of something even sightly more natural.

Why don't we improve languages so that they have pronouns for the stuff you're doing right there? Why don't comments get linked to line numbers in some external layer? And why would anyone commit commnented-out code to a repository?

The only answer to all of the above is that we are living in 2015 and not some future year. you work with what you have.

[+] overgard|10 years ago|reply
You might notice the commented code mentions CVS -- even though it's now on git. If they had done what you're suggesting, it would almost assuredly lead to a dead-end for someone trying to look up the code.
[+] V-2|10 years ago|reply
Yes, VCS doesn't give you a guarantee of storing your history (in an accessible way, I mean - "if a tree falls in a forest...") FOREVER, especially if you switch VCS systems on the way.

However, commenting some code out also doesn't mean it won't rot. Methods/procedures around it will be refactored, variables renamed - in long time perspective, in a different landscape it can become incomprehensible and useless anyway.

Just because some abstraction (version control) can be a bit leaky and one shouldn't rely on it absolutely, doesn't justify succumbing to some other illusion.

Commenting code out is not equal to hibernation and it doesn't make it immune to flowing time. You uncomment it one day, you discover it stinks to heaven high - especially if it was complex from the start - and isn't informative at all.

"But that's not the case here!" - well, guess what: version control failing to do what it's supposed to also isn't the case here, if you can still access commits from 1986, as someone pointed out: http://git.savannah.gnu.org/cgit/emacs.git/log/?ofs=124080

[+] mtanski|10 years ago|reply
There's an exception to every rule, like in this case. It drives me perpetually batty when folks blindly stick to process, best practices, convention even when it doesn't make sense.
[+] jonathaneunice|10 years ago|reply
Modern version control systems (Git, Hg, etc.) do a great job of storing file commits. They're very "flat," however. A file is a file is a file, and there's no good mechanism for storing tightly-linked documentation about the evolution of a project. Commented-out code is just one ad hoc way developers try to tack on that kind of information (often in clumsy, likely-to-be-obsoleted-or-forgotten ways).
[+] amichal|10 years ago|reply
For all the folks saying the comment itself is bad... Please explain why? All i can think is that adding a date and author might be nice so the current maintainers can give some thought to if we care about those old subtle bugs x decades later without digging through 3+ changes (guess) to version control systems.
[+] lmm|10 years ago|reply
Trying to use this to catch subtle bugs is the worst kind of "the implementation is the spec". If you can't document what it does except in executable code, you don't understand what it does, and you need to go back to requirements gathering and then write some tests for the important cases. Or, better still, write the actual code in a way that's readable and expresses the intent.
[+] pyre|10 years ago|reply
Well, the other question is does it even need to be in the code? Could there be a documentation / spec file somewhere that uses this code as the reference implementation, and just add a comment that says "see spec file" next to the code?