CppCoreGuidelines icon indicating copy to clipboard operation
CppCoreGuidelines copied to clipboard

NL.3 extention: remove commented code

Open nnaka opened this issue 7 years ago • 3 comments

This is something that I thought when reading NL.3: Keep comments crisp:

All too often, I see commented out code. As a somewhat extension to NL.3, comment code leads to maintainability pains and noise. This maybe a matter of opinion, so I'd be interested to hear thoughts, but I think code should either exist or not exist and not sit in a comment.

nnaka avatar Mar 11 '18 11:03 nnaka

Per our editor's discussion, the commented code can be useful in cases of "you used to be able to write this, but cannot anymore." But this is particularly annoying when developers comment out a page of code with the /* syntax.

This should be a separate rule, and contain the explanation that it can be especially confusing when future developers confuse the commented code with real code.

AndrewPardoe avatar Mar 26 '18 18:03 AndrewPardoe

Thanks for the feedback, @AndrewPardoe . Let me know if I can help draft a separate rule.

nnaka avatar Apr 08 '18 01:04 nnaka

TL;DR: I'm not entirely sold on this.

I believe there are several special cases where certain "code" remains in commented form, specifically to illustrate a certain important point. While of course in most other situations ugly plain disable-commenting of sometimes even larger blocks of code is done (undesirable, ultimately). So, in some situations such commenting should remain, whereas in other situations it should simply be removed immediately.

I'm also not so positive on a related argument "simply delete things - if they need to be brought back there's always SCM". After plain deletion other people won't even have the opportunity to know that there used to be something else available ===> would instead necessitate a one-liner placeholder reference comment providing some short history details.

Also, code commenting can be very useful "state machine" handling IMHO: e.g. when discovering unused #include:s, it can be a lot better to disable-comment them (transition to disabled-only state) rather than directly transitioning to outright removed-line state. That way an actual author/maintainer can still observe that something is disabled, realize that there is a bug (worst case: an entire sub functionality not yet implemented), and then properly "resurrect" the woefully missing functionality. Though of course in most cases the maintainer will realize that the disable-commented #include is unused, and simply transition it to deleted state.

So, a guidelines item IMHO should not unconditionally recommend plain "commented code" removal, but instead try to give some short decision-making/reasoning details.

andim2 avatar Mar 29 '25 18:03 andim2