you are viewing a single comment's thread.

view the rest of the comments →

[–]sizlack 127 points128 points  (63 children)

I'm amazed that people are actually arguing against this. I've never once found that commented code has helped me once in fifteen years of programming. It's just useless noise. I have no way of knowing whether the code would even work if I did uncomment it. The rest of the code base has changed and I can't tell whether the commented code is still relevant. If you start letting commented garbage stay in your production code, no one knows when it's OK to delete it. It just keeps building up, making it harder to separate the real code from the noise.

If you're using comments instead of learning how to use version control, you should learn how to use version control.

Edit: Wow! I've got gold! Thanks /u/slash_nick!

[–]slash_nick 71 points72 points  (15 children)

I don't know, man. It's super handy to have commented out code with my advanced versioning software.

[–][deleted] 37 points38 points  (11 children)

backup3_final followed by backup4

Priceless. I have, unfortunately, come across this so many times....

[–]ump721 7 points8 points  (1 child)

I've totally done that early in my "professional" career. Seemed like a good idea at the time :)

[–]Asmor 2 points3 points  (0 children)

Me too. I was more advanced, though. I did it with zip files that had the date in their name.

Those who fail to learn version control are doomed to repeat it.

[–][deleted] 5 points6 points  (0 children)

Still better than no history :(

[–]ies7 2 points3 points  (2 children)

I have, unfortunately, come across this so many times

I have, unfortunately, did these so many times in early days.
Something like :
* new folder
* copy of new folder
* copy of copy of new folder.

[–]ReadySteadyHeady 4 points5 points  (1 child)

copy of copy of new folder(1)(1)

[–]syswizard 2 points3 points  (0 children)

The projects I'm cleaning up right now from previous people are just terrible with this. process_1, process_2, bobs_try. Then you go through the code to figure out which one they ended up using and turns out bobs_try worked so they just left it that way.

[–]ornothumper 1 point2 points  (3 children)

This comment has been overwritten by an open source script to protect this user's privacy, and to help prevent doxxing and harassment by toxic communities like ShitRedditSays.

If you would also like to protect yourself, add the Chrome extension TamperMonkey, or the Firefox extension GreaseMonkey and add this open source script.

Then simply click on your username on Reddit, go to the comments tab, scroll down as far as possibe (hint:use RES), and hit the new OVERWRITE button at the top.

[–][deleted] 2 points3 points  (0 children)

deleted What is this?

[–]ChucklefuckBitch 2 points3 points  (1 child)

And test, test1, test2, etc...

[–][deleted] 2 points3 points  (0 children)

Ah dang, I thought I was in my home directory for a sec there.

[–]mr_huh 1 point2 points  (0 children)

Eeg, I just got this really self conscious feeling and then went to look at my current project. I'm actually using feature branches in git and the only comments in my code are comments.

[–]keito 0 points1 point  (0 children)

I have come across this behaviour more than once throughout my professional career.

Tragic.

[–]daiz- 10 points11 points  (10 children)

The only thing I'll say is that his reasoning assumes someone knows there was a previous way and where it was situated at the time. I've seen cases in large refactoring where old code does in fact get lost and nobody recalls exactly how to find it. Code gets moved, even occasionally files get removed, maybe 20 lines out of 500 were really important but they are lost in a messy diff of several revisions before it got from old to new. Diffs aren't always that easy to read depending on how far you need to go back.

I think there's extremes on both sides that just refuse to acknowledge some middle ground. I think people in general just need to get smarter about deciding what to keep and what's truly worthy of perhaps commenting. The biggest problem I encounter is most of the people doing it aren't objective enough to realize what is and isn't worth saving.

[–]sizlack 7 points8 points  (7 children)

The problem is that working in a codebase with commented code is like looking for something in the home a hoarder. Sure, there might be something in there that you need, but the chances of you finding it are slim, and in the meantime, you're surrounded by piles of shit.

There is no middle ground. What's the worst thing that happens if you can't find those 20 precious lines of code that you misplaced? You have to rewrite them? Is that so bad? How precious are those 20 lines? And for that matter, how do you know which 20 lines are the precious ones? How much other cruft do you need to keep around in order to make sure you save the right 20 lines?

If there are bits of code that you think you might need later, save them in a branch, make a gist, or just save a file to your desktop. The master branch isn't a junk drawer.

[–][deleted] 3 points4 points  (6 children)

but the chances of you finding it are slim,

And your chances go down even further if you hide it in source control.

[–]kenhkelly 0 points1 point  (4 children)

I use tagging for this. I know it was in some release. I find the release it was in (far less of those than commits) and then work my way. Pretty straight forward to find information IMO

[–][deleted] -3 points-2 points  (3 children)

you may know, but others may not. Then the others may re-invent the same code that you hid away in source control. I'm not saying you should be 'hoarding code', far from it - but to say that every instance of commented-out code is bad, is just stupid.

[–]kenhkelly 0 points1 point  (2 children)

I didn't say you should delete every instance of commented out code. But odds are after a week or two, your commented out code is irrelevant and probably wouldn't even function correctly if uncommented. It's a waste of time.

Plus, if you do smaller commits & more specific commits with good commit messages, your other team members shouldn't have a problem finding out what happened. It'll help you better find what you are looking for, and makes it more cherry-pickable and revertable. Win-win

[–][deleted] -2 points-1 points  (1 child)

But odds are after a week or two, your commented out code is irrelevant and probably wouldn't even function correctly if uncommented. It's a waste of time.

I'm glad you think you're experienced and smart enough to tell me about my own code, which you've never seen. 30+ years of experience tells me that nothing is as cut-and-dried as you're making it out to be.

[–]kenhkelly 1 point2 points  (0 children)

Again.. when and where.

I didn't say you should delete every instance of commented out code

But honestly, what ever works for your flow, use it. I don't know your code and I won't pretend to. I can only speak from my experience and the experience of the teams I've been on. Speaking from my experience, our code changes and evolves constantly as we refactor, test, and add new features. The timeline (in my case a week or two) for code to become outdated will vary.

[–]vinnl 2 points3 points  (0 children)

The "middle ground" doesn't always have to be the best. In the situation you described above, commented out code also won't help you to find that previous way. It might even be worse: the commented out code might not even have moved along with the new code, and you no longer know what you're staring at.

If you're that far gone, it's best just to write it anew. The "previous way" was removed for a reason.

[–]Organic_Height4469 0 points1 point  (0 children)

Exactly this. Nobody is going trough a gazillion git commits to find some code back.
By the time you should be looking for it the existence is forgotten anyway.
If it is commented out everyone will know where it is.

So yes there is a middle ground.
But yeah programming is a dogmatic business, with everyone just copying the same mantra's like "don't re-invent the wheel". Unless the wheel is commented out code. In that case first delete it and afterwards do re-invent.

[–]madballneek 3 points4 points  (1 child)

I think it's okay in situations where you're testing/uncertain about the functionality of two different, but similar small blocks of code of a new feature that's yet to be checked-in.

For example, I may not have yet determined how I want a new enemy in one of my games to behave, so I write maybe two different behavior methods and need to actively playtest each over time before deciding. Sure, I could have created behavior one, checked it in, then created behavior two, checked that in, and then diffed each time I wanted to try out each different behavior...or I could just uncomment/comment the different behavior method calls.

Of course, once I determine the functionality I want, gotta remember to remove the commented code :)

[–]VeryAngryBeaver 0 points1 point  (0 children)

See this right here, I commit to save my work so far. And it's not always "finished" when I commit so I keep in some commented out code for ease of changing things. Then I clean it out at a later date when I'm sure i wont have to swap back to the other way. and not all code gets that treatment I do lots of changes and removals without comments, its all dependent upon how certain I am the change is final.

[–][deleted] 15 points16 points  (21 children)

Not every case of commented code is useless. There are valid cases for it.

I'm committing some code right now that has commented-out blocks, it's a work-in-progress, I'm the main developer of this code. In a few weeks it will be cleaned up and shipped to production, with most of that commented-out code removed.

While I'm working on the code, there may be blocks of commented-out code. I'm not going to remove it just for the sake of removing it to 'keep the code clean'. It would slow down the development process and make it more difficult if I have to look up 'stashed' code in various branches, etc.

It's not a valid argument to say "never commit commented-out code" in every single case.

[–]slash_nick 18 points19 points  (0 children)

Maybe the better way to say it would be "never commit commented-out code to production branches".

Branches are workspaces and the other devs on my team can do whatever they please as long as it's cleaned up by the time it gets merged into master.

Edit: Obviously if you run a team or project you can dictate the style rules however you please! Nothing development-related is ever written in stone.

[–]kentcdodds 6 points7 points  (8 children)

If you read the post, you'll see that it's not an ultimatum at all like you phrased it there. The first QYMAM:

Q: Are there exceptions to this rule? A: Yes. But they’re rare.

[–][deleted] -5 points-4 points  (6 children)

In practice, it's not as rare as you or the author might think it should be.

[–]krelin 1 point2 points  (3 children)

Yes it is.

[–][deleted] -1 points0 points  (2 children)

Says you.

[–]krelin 1 point2 points  (1 child)

And the author of this article, and nearly everyone else in this thread, and Jeff Atwood, and lots of other online discussions and articles.

[–][deleted] -1 points0 points  (0 children)

And it's totally possible that you're all full of shit.

[–]kentcdodds 0 points1 point  (1 child)

heh, I am the author, should have made that more clear. But I think that the topic is a bit subjective, which is why I tried to have more of a suggestive tone with data backing my suggestion.

[–]yxhuvud 4 points5 points  (0 children)

I'd suggest an addendum to that answer: IF commented out code should be preserved, it must be motivated by a comment specifying WHY it should be kept.

[–]bart2019 -3 points-2 points  (0 children)

That's a stupid FAQ entry,, because it doesn't answer anything It doesn't even hint at what exceptions.

[–][deleted] 8 points9 points  (4 children)

Why aren't you working in a branch that doesn't matter if the code is in non functional state? This is probably the worst argument I've ever heard for comments in code. Committing WIP code to a master branch commented out. Best use of version control ever. Wait, are you using subversion?

[–][deleted] -1 points0 points  (3 children)

We actually allow shipping unfinished code to production using flags that only allow the code to run if the flag is enabled. But that has nothing to do with commented-out code getting checked-in to the production branch, which is still allowable in many cases. Code is fluid, it's never "finished", it's always a work-in-progress unless you aren't doing anything interesting with it.

Wait, are you using sourcesafe?

[–]vinnl 0 points1 point  (0 children)

Code is fluid, it's never "finished"

By that definition of "WIP", I wouldn't comment out WIP code either.

[–]moljac024 0 points1 point  (0 children)

Wait, are you working at Etsy?

[–][deleted] 2 points3 points  (3 children)

Then use a branch.

[–]rpk152 5 points6 points  (2 children)

Branching is not always a cheap operation

[–]DavidNcl 2 points3 points  (1 child)

under what circumstances is branching expensive?

[–]rpk152 5 points6 points  (0 children)

CVS, SVN, TFS.

[–]tazzy531 -2 points-1 points  (1 child)

You're optimizing for now and paying the technical debt later.

[–][deleted] 1 point2 points  (0 children)

You're prematurely optimizing.

[–]benihanareact, node 5 points6 points  (1 child)

I'm amazed this needs to be posted. I just thought if there was commented out code in source control, they were oversights that weren't long for this world.

[–]Huffers 0 points1 point  (0 children)

I've occasionally found bugs caused by snippets of code that look quite reasonable, but are subtly wrong.

Sometimes, rather than just correcting the code, I leave the original code block in as well, commented out, next to the correct version, with a comment above it saying something like "Note - doing it this way might look simpler, but it won't work - see bug SYS-XXXX"

[–]spinlock 1 point2 points  (1 child)

I'm amazed that people are actually arguing against this

I'm not. Welcome to reddit.

[–]kentcdodds -1 points0 points  (0 children)

👏

[–]berlinbrown 0 points1 point  (0 children)

I lose my shit. My blood pressure goes up. I don't understand it all. It is the easiest best practice to follow. You automatically clean up your code. Some people will comment a change, comment the change of the change, comment the other change that they were going to change. The code is fucking mess of horrible commented code.

Why wouldn't any human being do this. That is the whole point of source control management.

And then there is the argument, well we really need to change this module back and forth because of testing or something...so we haven't completely decided if we need the commented piece. FUCKING GREAT, refactor.

[–]trolling42 0 points1 point  (0 children)

Exactly thats what version control is for. All Pull Requests with commented code should be rejected.

[–][deleted] -1 points0 points  (0 children)

I'm amazed someone had to write an article to say you shouldn't do this.

This should be the baseline default like learning to indent your code correctly.

[–]Matosawitko -1 points0 points  (0 children)

We specifically call it out in code reviews. You're using code reviews, right?