Can commented-out code be valuable documentation?

Question

I wrote the following code:

if (boutique == null) {
    boutique = new Boutique();

    boutique.setSite(site);
    boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
    boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
    boutique.setNom(fluxBoutique.getNom());
    boutique.setSelected(false);
    boutique.setIdWebSC(fluxBoutique.getId());
    boutique.setDateModification(new Date());

    boutiqueDao.persist(boutique);
} else {
    boutique.setSite(site);
    boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
    boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
    boutique.setNom(fluxBoutique.getNom());
    //boutique.setSelected(false);
    boutique.setIdWebSC(fluxBoutique.getId());
    boutique.setDateModification(new Date());

    boutiqueDao.merge(boutique);
}

There is a commented-out line here. But I think it makes the code clearer, by making obvious what the difference is between if and else. The difference is even more noticeable with color highlighting .

Can commenting out code like this ever be a good idea?

score 266 · Answer 1 · edited Apr 05 '14 at 19:04

The biggest problem with this code is that you duplicated those 6 lines. Once you eliminate that duplication, that comment is useless.

If you create a boutiqueDao.mergeOrPersist method you can rewrite this as:

if (boutique == null) {
    boutique = new Boutique();
    boutique.setSelected(false);
}

boutique.setSite(site);
boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
boutique.setNom(fluxBoutique.getNom());
boutique.setIdWebSC(fluxBoutique.getId());
boutique.setDateModification(new Date());

boutiqueDao.mergeOrPersist(boutique);

Code that either creates or updates a certain object is common, so you should solve it once, for example by creating a mergeOrPersist method. You certainly should not duplicate all the assignment code for those two cases.

Many ORMs have built in support for this in some way. For example they might create a new row if the id is zero, and update an existing row if the id is not zero. The exact form depends on the ORM in question, and since I'm not familiar with the technology you're using, I can't help you with that.

If you don't want to create a mergeOrPersist method, you should eliminate the duplication in some other way, for example by introducing a isNewBoutique flag. That may not be pretty, but it's still much better than duplicating the whole assignment logic.

bool isNewBoutique = boutique == null;
if (isNewBoutique) {
    boutique = new Boutique();
    boutique.setSelected(false);
}

boutique.setSite(site);
boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE + fluxBoutique.getLogo());
boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE + fluxBoutique.getUrl());
boutique.setNom(fluxBoutique.getNom());
boutique.setIdWebSC(fluxBoutique.getId());
boutique.setDateModification(new Date());

if (isNewBoutique)
    boutiqueDao.persist(boutique);
else
    boutiqueDao.merge(boutique);

score 171 · Answer 2 · edited Apr 05 '14 at 19:25

171

This is an absolutely horrifying idea. It does not make clear what the intent is. Did the developer comment out the line by mistake? To test something? What's going on?!

Aside from the fact that I see 6 lines that are absolutely equal in both cases. Rather, you should prevent this code duplication. Then it will be clearer that in one case you additionally call setSelected.

edited Apr 05 '14 at 19:25

Dynamic

5,746
9
45
73

answered Mar 11 '13 at 09:41

JustAnotherUserYouMayKnowOrNot

1,211
1
7
5

9

Agreed. I'd assume seeing that that the commented-out line is old behaviour that has been removed. If a comment is needed, it should be in natural language, not code. – Jules Mar 12 '13 at 18:29
4

I agree entirely! I have recently spent hours on end trying to understand and clean up some applications that I've inherited that are almost completely unreadable because of this practice. It also includes code that has been disconnected from all other code but not removed! I believe that this a main purpose behind version control systems. It has comments as well as the changes that go with them. In the end, I have had at least 2 weeks of work added to my plate in great part because of this practice. – bsara Mar 15 '13 at 03:06
similar point of view in this post: [Don't pollute codebase with commented out code](http://reconvolution.blogspot.com/2017/12/dont-pollute-codebase-with-commented-out-code.html) – Nick Alexeev Jan 15 '18 at 01:19

score 122 · Answer 3 · edited Mar 11 '13 at 10:26

122

No, it's a terrible idea. Based on that piece of code the following thoughts come up to my mind:

This line is commented out because the developer was debugging it and forgot restore the line to its former state
This line is commented out because it once was part of the business logic, but it is no longer the case
This line is commented out because it caused performance problems on production and the developer wanted to see what the impact was on a production system

After seeing thousands of lines of commented out code, I'm now doing the only sensible thing when I see it: I immediately remove it.

There is no sensible reason to check in commented out code into a repository.

Also, your code uses a lot of duplication. I suggest you optimize that away for human readability as soon as possible.

edited Mar 11 '13 at 10:26

gnat

21,442
29
112
288

answered Mar 11 '13 at 09:47

Dibbeke

2,514
1
16
13

1

However I get rid of the duplicated code, it can hardly be seen as an optimization, I think. – Alexis Dufrenoy Mar 11 '13 at 09:49
23

it is an optimization for human readability – jk. Mar 11 '13 at 10:23
1

@jk: then I suggest we use another word. Like "readability", for example. "Optimization" already has a meaning. – Alexis Dufrenoy Mar 11 '13 at 10:39
12

@Traroth you can optimize for speed, memory use, power consumption or any other metric so I don't see that you can't optimize for readability (though as a metric it is a bit woollier) – jk. Mar 11 '13 at 10:44
3

Indeed, I meant human readability. Small hint here: your most important liability in programming is your code. So, less is truly more here. – Dibbeke Mar 11 '13 at 13:23
4

Software as a liability is also treated at http://c2.com/cgi/wiki?SoftwareAsLiability From there: _"Producing more code is not always a gain. Code is expensive to test and maintain, so if the same work can be done with less code that is a plus. Don't comment out dead code, just delete it. Commented out code goes stale and useless very fast, so you may as well delete it sooner rather than later, to loose the clutter. Keep good backups to make it easier."_ – ninjalj Mar 11 '13 at 23:53
That's just what I do. I tried often enough to answer the same questions you've listed. But then I discovered that the truth is, there is no reason to commit code commented-out without any other comment. Removing it never got me into any trouble, instead I save the time dealing with it. – Peopleware Mar 12 '13 at 08:19

score 116 · Accepted Answer · edited Apr 05 '14 at 18:23

116

Most of the answers focus on how to refactor this one specific case, but let me offer a general answer to why commented out code is usually bad:

First, commented out code isn't compiled. This is obvious, but it means that:

The code might not even work.
When the comment's dependencies change it will not obviously break.

Commented code is very much "dead code". The longer it sits there, the more it rots and provides less and less value to the next developer.

Second, the purpose is unclear. You really need a longer comment that provides context for why there are randomly commented lines. When I see just a commented line of code, I have to research how it got there just to understand why it got there. Who wrote it? What commit? What was the commit message/context? Etcetera.

Consider alternatives:

If the goal is the provide examples of using a function/api, then provide a unit test. Unit tests are real code, and will break when they are no longer correct.
If the purpose is to preserve a previous version of the code, use source control. I'd much rather checkout a previous version then toggle comments throughout the codebase to "revert" a change.
If the purpose is to maintain an alternate version of the same code, use source control (again). That is what branches are for, after all.
If the purpose is to clarify structure, consider how you can restructure the code to make it more obvious. Most of the other answers are good examples of how you might do this.

edited Apr 05 '14 at 18:23

Oded

53,326
19
166
181

answered Mar 12 '13 at 04:25

Chris Pitman

3,426
1
18
21

9

I think you are missing one important reason: Documentation: If the purpose is to document alternate design options an explanation of the alternative and especially the reason why it has been discarded should be provided instead of the original code. – Sarien Apr 05 '13 at 08:22
15

Design options are better explained in a human language than in a programming language. – Mark E. Haase Apr 05 '14 at 17:50
4

How would it be possible for subsequent developers taking over my project know that an alternative/previous/failed implementation exists in the source control? Are new developers expected to go through all version histories and change logs? Or it is a common practice to use comment to link to a previous commit's hash for every useful alternative implementation? If so, I never noticed. – Moobie Sep 05 '18 at 17:59
3

There is one caveat to this though. Sometimes, two equivalent code approaches can differ in performance and reability in a way that one is performant and the other is readable. In such a case, it's acceptable to _use_ the performant variant, but put the readable variant in comments so it's easier to understand the purpose of the code. Sometimes, a (commented) code line can be clearer than a verbose explanation. – Flater Jul 09 '19 at 08:41
@Flater Interesting point, but what happens when the code evolves? I can hardly imagine developers *maintaining* commented-out code. – Alexis Dufrenoy Nov 18 '19 at 09:16
Maybe to anticipate a coming change? Someone makes some evolution to the code, but an identified change will intervene in the forseable future. So the developer, who is in the swings of things, write the future evolution. But here also, version control offers a better alternative by using a branch. – Alexis Dufrenoy Nov 18 '19 at 09:22
@AlexisDufrenoy: The comment follows pseudocode standards (even if the commenter decided to use correct syntax), and the rules for updating it follow the standard of any text-based comment: change it when the behavior changes. If the comment-code is too complex to trivially change it, then it doesn't really serve its primary purpose of being an easier to understand version of the real code below it. – Flater Nov 18 '19 at 09:27

score 51 · Answer 5 · edited Apr 12 '17 at 07:31

51

I would just like to add to CodesInChaos's answer, by pointing out that you can refactor it further into small methods. Sharing common functionality by composition avoids the conditionals:

function fill(boutique) {    
  boutique.setSite(site);
  boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
  boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
  boutique.setNom(fluxBoutique.getNom());
  boutique.setIdWebSC(fluxBoutique.getId());
  boutique.setDateModification(new Date());
}    

function create() {
  boutique = new Boutique();      
  fill(boutique);
  boutique.setSelected(false);
  return boutiqueDao.persist(boutique);
}

function update(boutique) {
  fill(boutiquie);
  return boutiquieDao.merge(boutique); 
}

function createOrUpdate(boutique) {
  if (boutique == null) {
    return create();
  }
  return update(boutique);  
}

edited Apr 12 '17 at 07:31

Community

1

answered Mar 11 '13 at 13:33

Alexander Torstling

1,226
1
8
12

6

I think that's the cleanest suggestion here. – Alexis Dufrenoy Mar 11 '13 at 15:07
+1, and I would also add that the more you avoid passing around `null` objects, the better (I find this solution is a good example). – Nadir Sampaoli Mar 12 '13 at 09:00
I would pass `boutiqueDao` as inputs to `create` and `update`. – Happy Green Kid Naps Mar 12 '13 at 17:56
How can this work tho? How can you know when to call create and when to call update? The original code looks at boutique and knows if it needs to update or create. This just does nothing until you call create or update... – Lyrion Mar 13 '13 at 14:36
Lyrion: Trivial, I'll add that code aswell for clarity. – Alexander Torstling Mar 13 '13 at 15:51

Loren Pechtel · Answer 6 · 2013-03-13T03:18:22.290

26

While this is clearly not a good case for commented out code there is a situation that I think warrants it:

// The following code is obvious but does not work because of <x>
// <offending code>
<uglier answer that actually does work>

It's a warning to whoever sees it later that the obvious improvement isn't.

Edit: I'm talking about something small. If it's big you explain instead.

edited Mar 13 '13 at 03:18

answered Mar 12 '13 at 03:43

Loren Pechtel

3,371
24
19

5

What's wrong with `// the following part done like it is because of X`? Explain *why* you did something the way you did, not why you did *not* to it in some *particular* way. In your particular example, it eliminates the need for potentially a large block of commented-out code entirely. (I didn't downvote, but can certainly see why this would get downvoted.) – user Mar 12 '13 at 15:27
13

Michael, because it makes it clear to other coders (and yourself days/weeks/months later) that yes, you *did* try that cleaner/more clever approach, but no, it didn't work because of X, so they shouldn't bother trying it again. I think this is a completely legitimate approach and upvoted this sadly buried answer. – Garrett Albright Mar 13 '13 at 06:01
1

@GarrettAlbright: Thank you, I'm glad to see someone gets it. – Loren Pechtel Mar 13 '13 at 19:31
3

@LorenPechtel: Not only that, I was about writing more or less exactly the same. There are situations where it is very, very useful to quickly know which "obvious" solutions have been already tried without success and why they don't work. – JensG Apr 05 '14 at 19:24
I see no reason whatsoever to prefer cluttering your source code with comments that may or may not actually be relevant any more to writing automatic black box tests that ensure that any future dev that tries to "fix" or "optimize" it can verify that it still works as intended. comments don't compile and they don't get executed. you NEVER know if they are true or not, so in most cases they are worse than useless, they do actual harm. – sara Jan 26 '16 at 14:04
This is an unusual but legitimate use case. But I do concur with Michael that it's best to explain as briefly as possible. That means you should only show the **important** bits of the failing code (just enough for another developer to know how the code could be written in full). It's okay though, to show those bits properly formatted if they are too long or complex to show on a single line. – ADTC Oct 28 '17 at 11:46
3

Besides failed code with explanation, I would also comment out alternative implementations of the code which might be more efficient in a different production environment. For example, I have coded both a straightforward exponential time version of an algorithm, and a complex polynomial time version. But in current production, `n` is small, and the exponential algo is much faster. If somehow `n` changes later on, how would a future developer following up on my project know of a different implementation of the code buried deep within the hundreds of commits in source control? – Moobie Sep 05 '18 at 18:18

score 14 · Answer 7 · edited Apr 12 '17 at 07:31

In this specific example, I find the commented-out code very ambiguous, largely for the reasons outlined in Dibkke's answer. Others have suggested ways that you could refactor the code to avoid even the temptation to do this, though, if that's not possible for some reason (e.g., the lines are similar, but not quite close enough), I would appreciate a comment like:

// No need to unselect this boutique, because [WHATEVER]

However, I think there are some situations where leaving (or even adding commented out) code is not reprehensible. When using something like MATLAB or NumPY, one can often write equivalent code that either 1) iterates over an array, processing one element at a time or 2) operates the entire array at once. In some cases, the latter is much faster, but also a lot harder to read. If I replace some code with its vectorized equivalent, I embed the original code in a nearby comment, like this:

%% The vectorized code below does this:

% for ii in 1:N
%    for jj in 1:N
%      etc.

% but the matrix version runs ~15x faster on typical input (MK, 03/10/2013)

Obviously, one needs to take care that the two versions actually do the same thing and that the comment either stays in sync with the actual code or is removed if the code changes. Obviously, the usual caveats about premature optimization also apply...

"Obviously, one needs to take care that the two versions actually do the same thing and that the comment either stays in sync with..." - right there you explained why this is *not* a good idea. — sleske, Aug 27 '14 at 14:38
Well, that's a problem with all comments, right? Some vectorized code is sufficiently opaque that comments are worthwhile, and having an "unrolled" version can be handy for debugging. — Matt Krause, Aug 27 '14 at 17:07
True. Still, I'd try to keep the comment as brief as possible, not use full source code. Anyway, if you have an example, asking how to best make it readable would be a good question (here or on codereview.s.e). — sleske, Aug 28 '14 at 05:25
In your last case I'd keep both code variants as compilable code. — CodesInChaos, Feb 09 '15 at 16:27

Carl Smith · Answer 8 · 2019-07-09T10:28:23.957

The only time I've seen commented-out code that was useful was in config files, where the code for every option is provided, but commented out, making it easy to enable settings by just removing comment markers:

## Enable support for mouse input:
# enable_mouse = true

In this case, the commented-out code helps to document all of the available options, and how to use them. It is also conventional to use the default values throughout, so the code is also documenting the default settings.

score 7 · Answer 9 · answered Mar 11 '13 at 14:53

7

Generally speaking, code is only self-documenting to the person who wrote the code. If documentation is required, write documentation. It is unacceptable to expect a developer new to a source-code base will sit down read thousands of lines of code to attempt to figure out from a high-level what is happening.

In this case, the purpose of the commented-out line-of-code is to show the difference between two instances of duplicate code. Instead of attempting to subtely documenting the difference with a comment, rewrite the code so it makes sense. Then, if you still feel it is necessary to comment on the code, write an appropriate comment.

answered Mar 11 '13 at 14:53

Mike Van

224
1
3

2

This rings quite true. A lot of people (myself included) think their code is so awesome that it doesn't need documentation. However, everyone else in the world finds it gobbledygook unless it's thoroughly documented and commented. – Ryan Amos Mar 12 '13 at 03:04
"*code is only self-documenting to the person who wrote the code*" - Please pick a piece of complex, uncommented code you wrote a year ago and try tro understand it in a limited amount of time. You can't? Ooops. – JensG Apr 05 '14 at 19:26
I think it's a bit more nuanced. A lot of well written code is intelligible, and can be understood without comments. The issue is trying to figure out the bigger picture (even at a fairly local level) when you only have intricate details to go on. Comments are good for explaining non-obvious chunks of code, but when you have good docstrings, explaining what each function, class and module is actually for, you need a lot less help making sense of the implementation. – Carl Smith Jul 08 '19 at 18:02

score 4 · Answer 10 · answered Mar 11 '13 at 18:05

No, commented code gets stale, and is soon worse than worthless, it is often harmful, as it cements some aspect of implementation, along with all of the current assumptions.

Comments should include interface details and intended function; "intended function": can include, first we try this, then we try that, then we fail this way.

The programmers that I have seen try to leave things in comments are just in love with what they have written, don't want to lose it, even if it is not adding anything to the finished product.

score 2 · Answer 11 · answered Mar 11 '13 at 15:05

It can be in very rare cases, but not as you've done it. The other answers have pretty well hashed out the reasons for that.

One of the rare cases is a template RPM spec we use in my shop as a starting point for all new packages, largely to make sure nothing important is left out. Most, but not all of our packages have a tarball containing sources which has a standard name and is specified with a tag:

Name:           foomatic
Version:        3.14
 ...
Source0:        %{name}-%{version}.tar.gz

For packages without sources, we comment out the tag and put another comment above it to maintain the standard format and indicate that someone has stopped and thought about the problem as part of the development process:

Name:           barmatic
Version:        2.71
 ...
# This package has no sources.
# Source0:        %{name}-%{version}.tar.gz

You don't add code you know isn't going to be used because, as others have covered, it could be mistaken for something that belongs there. It can. however, be useful to add a comment explaining why code one might expect to be there is missing:

if ( condition ) {
  foo();
  // Under most other circumstances, we would do a bar() here, but
  // we can't because the quux isn't activated yet.  We might call
  // bletch() later to rectify the situation.
  baz();
}

arguably that comment isn't commented out code though. – jk. Mar 11 '13 at 15:48 — jk., Mar 11 '13 at 15:48
@jk: You are arguably correct. – Blrfl Mar 11 '13 at 15:53 — Blrfl, Mar 11 '13 at 15:53

score 0 · Answer 12 · answered Feb 12 '14 at 16:07

Commented-out code is not used by the application, so it needs to be accompanied by further comments stating why it is not being used. But within that context, there are situations where commented-out code can be useful.

What comes to my mind is a case where you solve a problem using a common and appealing approach, but then it turns out that the requirements of your actual problem are slightly different from that problem. Especially if your requirements turn out to require considerably more code, the temptation for maintainers to "optimize" the code using the old approach will likely be strong, but doing that will only bring the bug back. Keeping the "wrong" implementation around in the comments will help to dispel this, because you can use it to illustrate exactly why that approach is wrong in this situation.

This is not a situation that I can imagine occurring very often. Usually, it should be sufficient to explain things without including a sample "wrong" implementation. But I can imagine a case where that's not enough, so since the question is about whether it can be useful, yes, it can. Just not most of the time.

Sorry, but I fail to see any value of comment-out code. Code that is commented out is not used, hence it has no place in production code. — Vladimir Kocjancic, Apr 05 '14 at 19:08

Aura · Answer 13 · 2013-03-13T10:56:35.960

This doesn't look good buddy.

Commented code is...just not code. Code is for implementation of logic. Making a code more readable in itself is an art. As @CodesInChaos have suggested already that repetitive lines of code are not very good implementation of logic.

Do you really think that one true programmer will prefer readability over logical implementation. (by the way we have comments and 'complements' to put in our logical representation).

As far as I am concerned one should write a code for compiler and that's good - if 'it' understands that code. For human readability Comments are good, for the developers (in a long run), for people reusing that code (e.g. testers).

Otherwise you can try something more flexible here, something like

boutique.setSite(site) can be replaced with

setsiteof.boutique(site). There are different aspects and perspective of OOP through which you can increase readability.

While this code seems to be very appealing at first and one can think that he have found a way for human readability while compiler also does its job perfectly, and we all start following this practice it will lead to a fuzzy file which will become less readable in time and more complex as it will expand its way down.

*"As far as I am concerned one should write a code for compiler"* Oh please, don't. That's how you end up with monstrocities that look like they could be taken straight from the Obfuscated C Contest and the likes. Computers are binary, while humans use fuzzy logic (this goes double for pet owners, by the way). Computer time is next to free today (basically just electricity usage) whereas programmer time is comparatively very expensive. Write code for humans, and the compiler will understand it. Write code for the compiler without regard for humans, and you won't make many friends on the team. — user, Mar 12 '13 at 15:24
"*write code for a compiler*" - Actually you don't. The person you should have in mind is the one who's handed over the task to maintain your code. — JensG, Apr 05 '14 at 19:29

Can commented-out code be valuable documentation?

13 Answers13

Linked

Related