150

I've worked in shops that produce life critical software and I've dealt with commenting rules that were meant to keep the code readable and potentially save lives. In my experience though the requirement becomes a brain dead chore to be ticked off of a checklist and doesn't help me stay focused on writing understandable code. It also distracts my peer reviewer from having a more meaningful conversation with me about how to make the code easier to understand.

I've also graded student code that had no comments and seen why they should be marked down for neglecting them.

I understand that using good names, keeping structures simple, functions short, and modules focused will keep the code understandable enough that comments can be minimized.

I also understand that comments should explain why the code does what it does, not how.

Given all this is it even possible to write good coding standards that capture this idea? Ones that will be relevant in a peer review but won't turn into a mindless checklist activity that produces notes no more helpful than: "You forgot to comment on line 42".

An example of the kind of code this rule might require when treated as a line in a checklist:

/* Display an error message */
function display_error_message( $error_message )
{
    /* Display the error message */
    echo $error_message;

    /* Exit the application */
    exit();
}

/* -------------------------------------------------------------------- */

/* Check if the configuration file does not exist, then display an error */
/* message */
if ( !file_exists( 'C:/xampp/htdocs/essentials/configuration.ini' ) ) {
    /* Display an error message */
    display_error_message( 'Error: Configuration file not found. Application has stopped');
}

If it is possible to express this properly in a standards document, and it might simply not be, I'd like to capture an idea along these lines:

Consider a comment for every line, sequence, statement, section, structure, function, method, class, package, component, ... of code. Next consider renaming and simplifying to eliminate any need for that comment so you can delete it. Check in while comments are rare. Repeat until deadline. Then repeat some more

candied_orange
  • 102,279
  • 24
  • 197
  • 315
  • 21
    Comments are not for extended discussion; this conversation has been [moved to chat](http://chat.stackexchange.com/rooms/49010/discussion-on-question-by-candiedorange-coding-standard-for-clarity-comment-eve). – maple_shaft Nov 23 '16 at 17:04
  • 36
    “Comments are not for extended discussion” even truer for code comments. :) Those `/* Display an error message */` comments are horrible and actually impact readability. – Andrea Lazzarotto Nov 25 '16 at 17:25
  • We need our IDEs to have options for hiding comments. Maybe an option for both block and inline comment hiding separately. This way you have the best of both options. Put lots of comments in, but have the option to hide them to keep the code looking clean and uncluttered. – Doug.McFarlane Nov 25 '16 at 19:49
  • 1
    @Doug.McFarlane I thought that should exist for vim. Of course it does at both file and block levels - http://www.rtfm-sarl.ch/articles/hide-comments.html ! – Michael Durrant Nov 25 '16 at 21:00
  • 1
    Also toggling - http://stackoverflow.com/a/11842371/631619 – Michael Durrant Nov 25 '16 at 21:01
  • The coding standard should mention that comments are meant to **add information**. Check your examples above. There are many comments that add absolutely no information that is not clearly there already (e.g. if you don't know that exit(); exits the application, you have no business reviewing this code). – Tom Nov 27 '16 at 07:50

17 Answers17

175

Michael Durrant's answer is IMHO not bad, but it is not literally answering the question (as he admitted by himself), so I'll try to give an answer which does:

I also understand that comments should explain why the code does what it does, not how.

Given all this is it even possible to write good coding standards that capture this idea?

Obviously you can write a checklist for your code reviews, containing questions like

  • "if there is a comment, does it explain why the code does what it does?"
  • "is each line of the code - in its context - either self-explanatory enough that it does not need a comment, or if not, is it accompanied by a comment which closes that gap? Or (preferably) can the code be changed so it does not need a comment any more?"

If you like, you can call this a "coding standard" (or not, if you think the term coding standard should be reserved for a list of braindead rules, easy to answer if they are fulfilled or not, how the formatting of the code should look like, or where to declare variables). No one hinders you to have the focus of your checklist on semantical questions, instead of going the easy route and put only formal rules into it.

At the end of the day, you should be sure your team needs such a checklist. To apply such questions you need programmers with some experience as reviewers, and you need to find out if it will really improve the readability of the code in a team of experts. But that is something you have to work out together with your team.

Doc Brown
  • 199,015
  • 33
  • 367
  • 565
  • 49
    Who downvoted this? This is **the** answer for critical software development. This is a different world compared to where most programmers work. In these worlds, a coding standard that calls for that every single line of code to have its own comment is not the right answer. Neither is forgoing comments. OTOH, making comments subject to code review is exactly the right kind of standard for critical software. It makes for more reviewable and maintainable code, but this comes at a cost. That cost pales compared to multi-million dollar lawsuits that have resulted from poorly written software. – David Hammen Nov 20 '16 at 22:05
  • 4
    wise answer. if we could find genuine rules for coding, we would not need programmers. we could just have the machines code. it could happen! :( –  Nov 20 '16 at 22:29
  • 4
    @DavidHammen: thanks for your comment. Actually, I think this does not only apply to "critical software development". Every piece software which is maintained and evolved over years, often by different people, will benefit from beeing understandable for the next maintenance programmer. – Doc Brown Nov 21 '16 at 07:10
  • 5
    This is good advice for *any* coding, not just code maintained by different people. Even if you are the only one using a script/program, the future you who has to modify the code in a year's time will appreciate the clarity (and the time saved by not having to decipher opaque code). – Anthony Geoghegan Nov 21 '16 at 10:24
  • This is the correct answer. For me it's simply: given some code that is obtuse, it's good to add a comment explaining it but it's far better to refactor it to make it easy to understand. In rare situations this is not possible and more often there may not always be time. The main reason I use comments is when I have had to write seemingly silly or nonsensical statements to work-around bugs in a library. – JimmyJames Nov 21 '16 at 14:35
  • 5
    Could you edit "currently most upvoted answer" to something more objective? Not every reader is going to know the vote history of these answers. – candied_orange Nov 23 '16 at 01:05
  • @CandiedOrange: sure. Though, when I wrote this, I thought it was very unlikely that Michael's answer would leave its first place (and probably it won't). – Doc Brown Nov 23 '16 at 06:36
  • 1
    @DocBrown yes but for all anyone ignorant of the voting history knew you were talking about JacquesB's answer at the time. It just creates a puzzle to solve that distracts from your otherwise excellent answer. – candied_orange Nov 23 '16 at 06:40
  • I can understand why this is down voted. After reading it a few times it amounts to saying this; "You shouldn't drink and drive, but if you need to, this is how you drive drunk. Step 1: ..." – Andrew T Finnell Nov 23 '16 at 16:03
  • 1
    @AndrewTFinnell: sorry, but I don't understand your analogy. Can you explain what you mean? – Doc Brown Nov 23 '16 at 16:13
  • @DocBrown It is my understanding that you're proposing that a Coding Standard which requires certain comment rules, regardless of what those rules are, is not valuable as you'd need experienced engineers to assess that value. And if you had good engineers then what's the value of extraneous comments? If that is incorrect I apologize. So that being said, you then proceed to explain how to do the opposite of what you've suggested. With the suggestion being that it shouldn't be a Coding Standard. – Andrew T Finnell Nov 23 '16 at 16:21
  • @DocBrown The analogy I picked was in poor taste. I hoped it would convey sentiment and not literalness. In a way, this is great, because it proves the point about who derives value from which specific ordering of words prefixed with a `//`. It can't be qualified. – Andrew T Finnell Nov 23 '16 at 17:01
  • @AndrewTFinnell: ok, you had some problems with my last paragraph, maybe it was giving a wrong impression about what I meant. See my edit. Better now? – Doc Brown Nov 23 '16 at 17:04
  • @DocBrown I realize it'll *seem* insincere, but yes I do like what you reworded it to. In this question that is trying to determine whether one can accurately document a standard way of describing something, it's all in the minute details. Your change enhances your question tremendously in my mind. This sort of proves my previous point to others. You took the time to look at what you wrote and decided to change it. Others did not change their answers. Imagine if you had to be approved by 10 of the people writing on this question before your answer could even be posted. You may have moved on. – Andrew T Finnell Nov 23 '16 at 17:10
154

Major anti-pattern leading to poor quality code with less clarity

btw readers, the title was originally "comment every line of code?" and you can imagine the instinctive reaction of many of us, myself included. I later retitled to the longer more accurate title.

At first, reading your question I thought, yuch duplicate stuff in comments , but ok, maybe if you have enough strictures on reviews by multiple people... but then again: people makes mistakes, fail to notice inconsistencies, etc. and over time there will end up being differences. On reflection I think the problem is much larger:

Developers will tend to write worse code. They will use more cryptic names and structures because 'its explained in the comment - see!'.

Consider:

living_room_set = tables + chairs.

Now consider:

set = tbls+chrs # Make the set equal to the table plus the chairs

You not only have both more cryptic names and more to read, you also have to look back-and-forth, look at code, what is a set? look left at code, look right at comments, what are tbls, look left at code, look right at comments, what are chrs, look right at comment. Yuch!

Summary

Develop or enhance a commenting standard if you are forced to in your current position as a developer (As a former COBOL programming I've sure seen big ones!) but spent as much of your time, energy and passion arguing against the commenting anti-pattern using the arguments:

  • Code and Comments get out of sync with each other when only one is changed

  • Comments may describe what one person thinks but may be wrong and thus cover up bugs

  • Code becomes harder to read

  • Good code becomes harder to write

  • Tendency to have use more cryptic names

  • Excessive characters to read in code and comments

  • Different editors use different styles

  • Requires higher English language proficiency than just coding

  • Takes time and resources and subtracts from long-term value *

Plus argue for better code without such comments,- in fact have a standard(!) - all variable names must be full_english_words - there's one! So use that 'standards' energy towards the standards of well written code and tests.

One of the two hard problems is naming things - don't side skip the problem with comments.

Given that one of the other problems is premature optimization and that optimization in general can leads to obscure 'why this way' code, this is one area where explanatory comments for apparently poor code may be beneficial although the above caveats still apply.

* tomorrows code

Michael Durrant
  • 13,101
  • 5
  • 34
  • 60
  • 8
    I don't believe you answered the question. The question is not about having comments on every line. It is asking how to tell developers to comment code without causing them to comment every line. – hichris123 Nov 20 '16 at 22:58
  • 7
    Yeah I am not answering the specific standards question but instead trying to help folks not go down that route. So this may not be the answer to the specific question _but_ does contain valuable advice that is hard to lay out in a comment. – Michael Durrant Nov 21 '16 at 00:34
  • Added a little bit to allow for such standards if forced but to argue against them. – Michael Durrant Nov 21 '16 at 01:41
  • 1
    What is the other hard problem? – David says Reinstate Monica Nov 21 '16 at 16:13
  • 21
    @DavidGrinberg [cache invalidation and off-by-one errors](http://martinfowler.com/bliki/TwoHardThings.html). – Eric King Nov 21 '16 at 17:07
  • I once worked for a place that mandated all vowels be removed from variable names because 20 years ago the compiler mandated 8 character variable names. Even though for 10 years you could have bigger, they didn't want to break the consistency going forward. Unless it was to tack on the Hungarian, then you could go over... I'm so glad I'm not coding there anymore... – corsiKa Nov 21 '16 at 17:12
  • Fun fact: In assembly language, most (but still not all) lines should have comments explaining the semantic meaning (and not repeating what the instruction mnemonic + operands already say). Sometimes there's still nothing to say about obvious stuff like `call printf`, except maybe to summarize the args that were set up earlier. Using macros to make up meaningful-name aliases for registers would have different (worse) downsides than needing to keep a lot of comments in sync. – Peter Cordes Nov 22 '16 at 01:42
  • In my experience I see all the time that the freedom not to comment leads to poor quality code with _less_ clarity, the obvious reason being that not having to explicitly declare the intention of the code allows developers to throw in code that does something that they themselves are far from sure that is what the code should do there. And I have never ever seen one case where freedom not to comment actually resulted in better code (I mean in practice, not in a book). Not being religious here, just applying my honest observation of reality. BTW have you a source for the anti-pattern claim ? – SantiBailors Nov 22 '16 at 10:54
  • I have about the same years of work experience as you and I very consistently observed exactly the opposite, be it where I worked, where friends of mine worked or at the customers where I was sent. However I see from your tone that the intentions of my comments did not come across right so let's not elaborate further. But a sincere thanks to you, because - even though I cannot imagine being so lucky as to find 4 companies in a row that believe in quality and practice good code _for real_ (I didn't get the checkbox metaphor, sorry) - thinking that they are there is really refreshing. – SantiBailors Nov 22 '16 at 16:22
  • no problem. I'm lucky enough to work in newer smaller start-up style companies for the last few years so that is a huge influence I'm sure :) – Michael Durrant Nov 22 '16 at 22:34
  • @hichris123, I love your one sentence summary of the question, but I factually didn't get that idea **at all** out of reading the actual question. If you really think that's what it's about I think you should suggest an edit. – Wildcard Nov 23 '16 at 02:30
  • Downvoted for not answering the question. – user253751 Nov 23 '16 at 05:03
  • I believe the person answering this is being polite by agreeing that they didn't answer the question. This answer does address his concerns. The issue here is that we're all trying to deal with subjective vs. objective tests. – Andrew T Finnell Nov 23 '16 at 15:29
  • fyi http://softwareengineering.stackexchange.com/questions/145574/recommendations-for-teaching-junior-programmers-good-coding-style/145595#145595 – Michael Durrant Nov 23 '16 at 20:32
  • 1
    Each of your reasons to avoid comments can be countered but StackExchange doesn't allow long enough comments (heh) for me to address them all here. Simply: overcommenting is like voter fraud: it does happen, but very rarely; when is the last time you actually saw it happen? Writing good comments should not be simply an afterthought; if done correctly, it helps the development process. Yes, it takes resources, but the payoff is better code. You will never be able to get the same bang for your buck by sacrificing comments for a better variable naming standard. Yes, commenting every line is nuts. – kmoser Nov 27 '16 at 02:10
  • 1) overcommenting. yes I have seen 2 or 3 folks do this. The mountains they wrote made up for their rarity. If the development process cares about good comments.... it can care enough to make those comments part of the code itself. Rules against comments without corresponding care and attention paid to code reviews and high quality code is not good. Having both in place in what I think folks should strive for. – Michael Durrant Nov 28 '16 at 19:28
  • Longer names aren't always better. Sometimes, abbreviation is a boon. `foo.v = PI * pow(foo.r,2) * foo.h` in a pile of code is easier to scan and understand than `volume_of_foo = ratio_of_circumference_to_diameter * raise_to_power(radius_of_foo, power=2) * height_of_foo`. – Brandin Feb 27 '17 at 07:50
  • 'pow' hmm lets us, ah raise_to_power, got it. and `h`?... ah yes height. I prefer to leave short-term memory free for other stuff. – Michael Durrant Mar 27 '17 at 12:43
  • May I conclude what we need? I think a good document is needed if comment of each line is not necessary. Finally, it becomes, we try our best and use all tools (drawing, video, doc, comments, etc) to explain our code, right? – skytree May 23 '23 at 05:12
  • Lots of good comments here. Not seeing solutions to the fact that not matter how good or bad comments are... they get out of date because they are manually maintained. Then they can have negative value. Think about the future not just the moment. – Michael Durrant May 23 '23 at 09:42
24

I don't think a coding standards document is the place to specify what is already common sense. The purpose of a coding standard is to guarantee consistence (and avoid arguments) about issues where reasonable people might disagree and there is no single obvious right answer, e.g. naming conventions, indentation etc.

Such commenting as in your example is objectively useless and might be due to an inexperienced developer, or a developer used to work with a build system which mechanically checks for the the presence of comments (a really bad idea, but it exists). In both cases the solution is education, possibly through code reviews.

If you start adding all kinds of "best practices" to the coding standard, you will just end up repeating which is already stated in multiple books. Just buy "Clean Code", "Code Complete" and "The Pragmatic Programmer" to the developer in question, and keep your coding standard lean.

martin
  • 363
  • 1
  • 2
  • 6
JacquesB
  • 57,310
  • 21
  • 127
  • 176
  • 64
    One of the many things I have learned after over 40 years in this racket is that common sense is not at all common. – John R. Strohm Nov 20 '16 at 18:25
  • 11
    There is no such thing as common sense. – whatsisname Nov 20 '16 at 18:41
  • 1
    _in_programming_ no.. real world oh yeah but its just a term for real world expereienced knowledge – Michael Durrant Nov 20 '16 at 22:15
  • @JohnR.Strohm: My experience have been much more positive than yours, but I have also seen how the the use of common sense can be actively discouraged by mechanically enforcing rules. – JacquesB Nov 22 '16 at 14:10
  • I am in 99.9% agreement with this answer. The Coding Standard is meant to be the thing developers point to when a debate happens at Code Review. Code reviewing is meant to enrich the product not start wars. So many wars I have seen instigated by Code Reviews with no Coding Standard. I dare say that if you cannot automate the checking of the items in the coding standard it should be in there. A Code review is the time to impart 'common sense' to other less experienced developers. It's not the time to fight over the naming of variables. linux/kernel/gen_init_cpio.c line 335. – Andrew T Finnell Nov 23 '16 at 14:37
  • The same file, Linus himself, on line 322 he did NOT put a space between the function and the open paren. Then on line 328, he did put a space. The essence of these two lines of code has wasted days and days of my life in Code Reviews that do not improve quality, because someone wanted to argument over a space. – Andrew T Finnell Nov 23 '16 at 14:40
20

It is not possible. What you are essentially trying to do is nearly to mechanize good judgment.

When writing critical software such as for life supporting medical systems, huge amounts of checklists and whatnot are virtually inevitable. Not only do even smart people make mistakes, a lot of the software for these devices is written by people that aren't very good at their jobs, so the 'system' has to be able to safeguard against sloppy work, making it safe at the expense of marketability.

Your best option is to leave out the rigorous Coding Standard for commenting and lead the team by example. Show other's what good comments look like by striving to produce good quality code that is appropriately commented. Instead of trying to find the ultimate way of describing how to write comments (which are themselves more often than not a judgement call) show the others what you mean on a daily basis with your own code reviews. As the other members read your code they'll follow suit if they feel it's useful. And if they can't then no Standard would help them write better comments to begin with.

Andrew T Finnell
  • 3,112
  • 17
  • 18
whatsisname
  • 27,463
  • 14
  • 73
  • 93
  • 1
    +1 for "You are trying to mechanize good judgment," -1 for "Your only option is to just hope for the best," and that leaves me not voting. *Educating* and *training* your team is an option, also. Standards *can* allow for judgment. – Wildcard Nov 23 '16 at 02:35
  • 1
    @Wildcard Perhaps you could explain how a Standard can allow for judgement? Wouldn't that be a Guide at that point? A Standard is "an idea or thing used as a measure, norm, or model in comparative evaluations." How can one use something inherently subjective, so subjective it depends on who's reading it, as a measurement of quality? – Andrew T Finnell Nov 23 '16 at 15:52
  • 1
    @Wildcard: The problem is that when you enter into regulated industries, the process reigns supreme over everything, having a process is more important than whether the process is worthwhile or not. Everything has to boil down to checkboxes on some form, every checkbox has to be specific and verifiable, and the more lee-way you have in a checkbox the more risk you run of getting hassled in an audit. – whatsisname Nov 23 '16 at 17:51
  • It's a strange world, and keep in mind the rules are written by bureaucrats with little knowledge of the software domain, and you often end up with systems that exist to justify and require themselves rather than being a means to an end to ensure quality products. – whatsisname Nov 23 '16 at 17:52
  • Educating and training your team isn't an option from the aspect of the quality system, as you have no guarantee the team actually learns or applies anything, and no objective measure for which to have a checkbox to check that they are applying what they supposedly learned. – whatsisname Nov 23 '16 at 17:54
  • 1
    @whatsisname I'm very happy there's someone else that understands. Your responses are written more elegantly and precise than I could have ever strived for. The statement `with systems that exist to justify and require themselves rather than being a means to an end to ensure quality products` is EXACTLY what i was attempting to illustrate. Thank you for finding my words for me. I truly mean it. We have to separate the idea of inducing quality from the idea of following a process as they aren't the same. This is why we have QA, Stackoverflow, and very forgiving customers. – Andrew T Finnell Nov 23 '16 at 18:10
  • I upvoted the modified answer. Very nice. – Wildcard Nov 23 '16 at 19:51
  • @wildcard Thanks! I really liked his answer and didn't want it to be lost in the weeds of a single statement. – Andrew T Finnell Nov 25 '16 at 20:12
13

Update

My response in quotes for emphasis:

It is my belief the answer that states the comments should not be addressed in Coding Standards and then lists a set of defensive questions to fight it, is the only correct answer.

The issue here is that a Coding Standard is just that, a Standard. Extremely subjective ideas should not be in a Coding Standard. It can be a in a Best Practices guide, but that guide cannot be used against a developer during Code Review. In my personal opinion, a Coding Standard should be as close to Automated as possible. There is so much time wasted in Code Reviews arguing over naming, spacing, tabs, brackets, comments, etc. etc. when ALL of it can be automated. Even the answer about tables and chairs can be automated. LINT'ers allow for dictionaries, Capitalization Checks per concept (Variable, Function, Method, Class, etc.).

Even JavaDoc checking can be implemented by a Robot LINT'er before a Pull Request is accepted. A lot of Open Source projects do this exact thing. You submit your Pull Request, the code is built with a Travis-CI file, including Static Analysis, and it must pass all the Coding Standards which can be objectively expressed. No human chimes in about 'doing it incorrectly' or not 'providing value' with a comment, or the wrong way to name variables, et el. Those discussions provide no value and are best left to a third-party robot which can take the brunt of our emotional coding.

To actually answer the question we would have to address how to write a Standard which addresses how to answer the following question: Did this comment provide value? A Coding Standard can't possibly dictate the 'value' of a comment. Therefor it becomes necessary for a human to go through that checklist. The mere mentioning of comments in a Coding Standard creates a checklist which the Original Poster is wanting to avoid.

That's why comments are usually not processed by the compiler and stripped out. Their value cannot be determined. Is the comment in question valuable; yes or no?. Answering this question is NP-hard. Only humans stand a chance at properly answering it, and even then it can only be answered at the time it's read, by the person who is reading it; where the value of that comment is affected by the weather, his or her home life, the last meeting they just attended and did not end well, the time of day, amount of coffee they've had. I trust the picture is becoming more clear.

How can it ever possibly be expressed properly in any Standard? A Standard isn't useful unless it can be applied consistently and fairly, where fair is more about objectiveness not emotional.

I contest that a Coding Standard should remain as Objective as possible. The way variables are named IS objective. They can easily be checked against a dictionary for proper spelling, grammatical structure, and casing. Anything beyond that is a "pissing match" that is won by the person with the most power or by "brow beating". Something I, personally, struggle with NOT doing.

When I comment, I always comment talking to my future self in the third person. If I come back to this code in 5 years, what would I need to know? What would be helpful, what would be confusing, and what would be out of date with the code? There's a different between documenting code to generate a searchable public API and commenting code which provides value to an unknown third-party, even if that third-party is yourself.

Here is a good litmus test. If you were the ONLY one on the project. You knew you'd be the only one on the project. What would be in your coding standard? You'd want your code to be clean, self explanatory, and understandable to yourself in the future. Would you have a code review with yourself about why you didn't put a comment on every line? Would you review every single comment you created on the 100 files you checked in? If not, then why force others?

Something I believe is missed in these discussions is that the Future You is also a developer on this project. When asking about value, tomorrow's you is also a person that can derive value from the comment. So team size, in my opinion doesn't matter. Team experience doesn't matter, it changes too often.

No amount of comment code reviewing this stop a pace maker from crashing and killing a patient. Once you talk about a comment which affects code, you're now talking about the code not the comment. If all it takes is a missing comment to kill someone, there is something else that smells in the process.


The solution to this type of rigorous way of coding has already been provided as a methodology to writing software itself. And it has nothing to do with comments. The problem with comments is that they have NO impact on the way the product ultimately works. The best comments in the world cannot keep software from crashing when embedded within a pace maker. Or when measuring the electrical signals with a portable EKG.

We have two types of comments:

Machine Readable Comments

Comment styles such as Javadoc, JSDoc, Doxygen, etc. are all ways of commenting the public interface a set of code provides. That interface may only be used by single other developer (Proprietary code for a two person team), an unknown number of developers (e.g. JMS), or for an entire department. This code can be read by an automated process which then produces a different way of reading those comments, ala HTML, PDF, and such.

This type of comment is easy to create a standard for. It becomes an objective process of ensuring every publically invoke-able method, function, class contains the required comments. Headers, parameters, description et. el. This is to ensure that it's easy for another team to find and use the code.

I'm doing something that looks crazy, but it's really not

These comments are here to help other's see WHY this code was written a certain way. Perhaps there is a numerical error in the processors the code is running on and it always rounds down, yet developers typically deal with code which rounds up. So, we comment to ensure that a developer touching the code understands why the current context is doing something would normally seem unreasonable, but in reality was written that way on purpose.

This type of code is what causes so many problems. It typically goes uncommented and is later found by a new developer and 'fixed.' Thus breaking everything. Even then, the comments are only there to explain the WHY not to actually prevent anything from breaking.

Comments cannot be relied upon

Comments are ultimately useless and cannot be trusted. Comments do not normally change the way programs run. And if they do, then your process is causing more problems then it should. Comments are afterthoughts and can never be anything but. The code is all that matters as that is all that's processed by the computer.

This may sound asinine, but bear with me. Which of these two lines really matters?

// We don't divide by 0 in order to stop crashes.

return 1 / n;

In this example all that matters is that we have no idea what 'n' is, there isn't any check for n being 0 AND even if there was, nothing stops a developer from putting n = 0 AFTER the check for 0. Thus, the comment is useless and nothing automated can ever catch this. No standard can catch this. The comment, while pretty (to some) has no bearing on the outcome of the product.

Test Driven Development

What does have an outcome on the product? Industries in which the code being written can literally save or kill someone has to be rigorously checked. This is done through code reviews, code reviews, testing, testing, code reviews, unit tests, integration tests, trials, staging, months of testing, code reviews, and single person trials, staging, code reviews, testing, and then maybe finally going into production. Comments have nothing to do with any of this.

I would rather code that had NO comments, had a specification, had unit tests which verified the spec, studies of the outcomes of running the code on the production device, then well documented code that had never been tested, nor had anything to compare the code against.

Documentation is nice when you're trying to figure out WHY someone did something a certain way, however, I've found through the years that documentation is typically used to explain why something 'clever' was done, when it really didn't need to be written that way.

Conclusion

If you work at a company which REQUIRES every line be commented I GUARANTEE at least two of the software engineers on the project have already written an auto-document program in Perl, Lisp, or Python which determines the general idea of what the line is doing, then adds a comment above that line. Because this is possible to do, it means the comments are useless. Find the engineers who have written these scripts to automatically document the code and use them as evidence to why the 'Comment on Every Line' is wasting time, providing no value, and potentially hurting.

On an aside, I was helping a close friend with a programming assignment. His teacher had set a requirement that every line had to be documented. So I can see where this thought process would come from. Just ask yourself, what are you trying to do, and is this the right thing? Then ask yourself; Is there any way to 'game' the system with this process? If there is, then is it really adding any value? One cannot automatically write unit tests which tests that code meets a certain specification, and if they possibly could, that wouldn't be a bad thing.

If a device has to work under certain conditions because it'll be inside a human, the only way of ensuring it's not going to kill them is years of testing, peer review, trials, and then NEVER EVER changing the code again. This is why NASA is / was still using such old hardware and software. When it comes to life or death you don't just 'make a little change and check it in.'

Comments have nothing to do with saving lives. Comments are for humans, humans make mistakes, even when writing comments. Don't trust the humans. Ergo, don't trust the comments. Comments are not your solution.

Andrew T Finnell
  • 3,112
  • 17
  • 18
  • 3
    _The problem with comments is that they have NO impact on the way the product ultimately works._ well if the human part of reading and writing code counts I think that *does* affect how it *ultimately* works, but not how the code *executes* when finally written, yes. I would say THE problem with code is simply that it gets out of date and then is *worse* than no code! – Michael Durrant Nov 20 '16 at 21:05
  • 1
    Here here, we had a timekeeper who wanted daily reports for what we did so he could over optimize our work. In anycase we were a bit angry about the fact that we needd to spend 15 minutes each day filling his form so we automated this by filling in garbage from our logs. – joojaa Nov 20 '16 at 21:16
  • @MichaelDurrant Perhaps, however, I would argue that the problem with code is that people keep changing it. This is why code which is used in mission critical systems is so difficult to change. select isn't broken.. there's no reason to keep messing with it. The person posting the Question specifically mentioned an Environment which is NOT a common situation. Their Use Case or rather Requirements are NOT typical. He specifically stated their products could be used to save lives, or take them if poor code is written. Perhaps I dreamed that up.. – Andrew T Finnell Nov 20 '16 at 22:25
  • 2
    There's an extra problem with the comment *"We don't divide by 0 in order to stop crashes."* It's not clear grammatically if the "in order to" clause applies to the action of division, or to its negation. If you use "We avoid dividing by 0 in order to..." you avoid this ambiguity. :) – Wildcard Nov 23 '16 at 02:39
  • 1
    "don't divide by zero in order to stop crashes" actually tells me a lot about the mindset of the person who wrote it. *You do not code to avoid crashes!* You code so that the code is correct, and not crashing is just a small part of being correct. If the calculation for the amount of medicine to inject ends up dividing by zero, you don't return some dummy value like 99999... – user253751 Nov 23 '16 at 05:05
  • @immibis I don't understand what you're attempting to say. Perhaps you can clarify. In mission critical applications you most certainly code to avoid crashes. Crashes are accepted as being 'ok' by too many developers now a days. We have auto restarting, auto crash log submissions, backup data before the crash, buffer overflow detection, and so on. There are certain scenarios in which a program must not crash. Some of the newer technology is too sensitive to movement. So imagine having a hardware box in a moving vehicle calculating important math. It is required to not crash and be accurate. – Andrew T Finnell Nov 23 '16 at 14:25
  • @MichaelDurrant The human part of reading and writing doesn't count. The computer doesn't take into consideration the attention to detail the author had in writing the code. It does what it's told. We're taking the assumption that ALL humans would derive the same, deterministic value from any given comment. Comments are aids, helpers, reminders, and flags. The commands the computer was told to run is the end all.Your argument is close to; "A Manager who has a happy wife and happy life is responsible for creating quality correct code because a developer isn't stressed by that Manager". – Andrew T Finnell Nov 23 '16 at 15:50
  • 1
    @AndrewTFinnell I mean that if you sometimes get a division by zero error, adding "if(divisor == 0) return ; // avoid division by zero" doesn't make your code any more correct. Instead you need to find why the divisor is zero and fix that. Some algorithms are designed so the zero divisor is unavoidable, but they're the exception (And in *that* case you can return a value of "result cannot be computed"). – user253751 Nov 23 '16 at 20:53
  • Please don't forget to mention that sometimes the way to deal with division by zero and other such problems is to crash the use case but save the application by recovering and displaying an error message that explains to the user what happened to their use case. That's what my OS's calculator always does. – candied_orange Nov 25 '16 at 14:04
13

There are two different things you allude to when you talk about comments. They're not the same, and the distinction is important.

Documentation tells you about the outward-facing bits of a piece of code - its interface.

Typically tooling will allow you to read them in a 'standalone' fashion, i.e. you're not looking at the underlying code at the same time.

All the interface should be documented (e.g. each method, excluding private methods), and it should describe inputs, outputs, and any other expectations, limitations, etc., especially things which can't be expressed through constraints, tests, etc. (Exactly how much detail and where it goes depends on the project).

Good documentation allows the potential consumer to decide whether, when, and how to use the code.

Comments in source code have a different purpose. They are there for people looking at the source code. They primarily describe the implementation.

Comments are always read in amongst the underlying code.

Comments should explain surprising decisions or complex algorithms, or of options not taken (and reasoning). They are there so future developers can understand the thought processes of their predecessors, and have at hand information which they otherwise might overlook or spend much time seeking, e.g.

# 'map' outperforms 'for' here by a factor of 10  

// This line works around an obscure bug in IE10, see issue #12053  

-- We are going to recursively find the parents up to and including the root node.
-- We will then calculate the number of steps from leaf node to root node.
-- We first use a WITH clause to get the ID of the root node.

You cannot infer anything from the absence of a comment, and the comment may of course be as wrong as the surrounding code or more so. Judgement must be exercised both on the part of the author and on the part of the reader.

Commenting every line is clearly more work of questionable additional value, which comes with an opportunity cost. If you have a rule that says every line must be commented, you will get that, but at the expense of important parts being commented well.

But it's worse than that: the purpose of a comment is defeated if every line is commented. Comments become noise which make code hard to read: obscuring rather than clarifying. Moreover, indiscriminate commenting makes the truly important comments harder to spot.

Neither comments nor documentation provide any sort of measure or guarantee of the quality of code they describe; they are not a replacement for genuine QA. Their purpose is forward-looking, i.e. in the hope that they will help those who interact with it avoid making mistakes.

In summary, your coding standards can and should require documentation (automated checks help spot undocumented functions/methods, but humans still need to check if the documentation is any good). Comments are a judgement call, and your style guide should acknowledge this. Those who never comment, and those who comment without thinking should expect to be challenged.

candied_orange
  • 102,279
  • 24
  • 197
  • 315
user52889
  • 1,683
  • 11
  • 13
  • Document the public API is a good point but I really like explain surprising decisions. It is very nice if violations of the principle of least astonishment at least comes with a justification. +1 – candied_orange Nov 21 '16 at 06:53
  • 1
    +1 for extraneous comments as noise. Keep signal to noise high. – sq33G Nov 21 '16 at 15:37
  • +1 especially for _the purpose of a comment is defeated if every line is commented._ Too bad this might be (and often is) used as an excuse not to comment at all, but it is very right anyway, for the reasons you mentioned plus because if every line is commented most people will automatically and understandably start ignoring _all_ the comments. – SantiBailors Nov 24 '16 at 08:51
10

You can't codify a commenting standard, because you can't know what the important comment will be in advance.

But you still want to make sure the code is commented correctly, since this is life critical code. The solution is to have a standard for the code review - require that code review comments on understandability.

This won't guarantee that it won't turn into a meaningless check list, but it will at least keep it from making the code harder to read. And has the possibility of making it better.

This requires a culture where a code review isn't itself a meaningless gesture, where getting your code sent back as hard to read is a good thing, and not an insult or annoyance. Just as important, one where saying it was unclear, isn't seen as a failure of the reader.

Unreadable code is, to a certain extent, unavoidable. Because the writer is immersed in the context, and will see something as obvious when it's only obvious if you know x, y, or z.

So, you won't be able to have a rule that says give good feedback, but you can spot check the reviews. Even a non-programmer manager could do so -- because the real question isn't was the code written to be readable, but was the code actually readable, which can only be decided by someone who read it.

jmoreno
  • 10,640
  • 1
  • 31
  • 48
  • 7
    Re *You can't codify a commenting standard* -- Sure you can. You make comments subject to code review, and that code review item that says the comments don't explain the code must be addressed. This is an expense, an expense that most programming environments don't want to pay. It pays off with regard to critical software. – David Hammen Nov 20 '16 at 22:15
  • "...when it's only obvious if you know x, y, or z" You could specify the amount of knowledge {x,y,z} that needs to be known in advance in order to see something as obvious and then (kind of mechanically) you can check if this holds true for the current code. Where not, add comments until it is. – NoDataDumpNoContribution Nov 21 '16 at 16:01
  • 1
    @DavidHammen: that isn't a standard for the comment, it's a standard for the review. Which is what I went on to suggest as the solution. You can't say it has to have be a certain size, or use a particular syntax or even usefully require a comment in the first place (or you get "add one to i" type comments). You CAN say that the **reviewer** must comment on the code and comments. As you say you can require that issues raised by the code review be addressed. But the onus for this HAS to be on the reviewer to do a good job. Only the reviewer can judge whether the code and comment are enough. – jmoreno Nov 22 '16 at 02:46
  • @DavidHammen The people picked for the review make the judgement call? And when they leave? If new people don't think the same way or speak English? If the 'Reviewers' leave? Your suggestion puts a few developers on top of an Ivory Tower, leading to dissent, distrust, and a break down of communication. Comments are meant to provide insight or clarification for those that can use it. Perhaps Joe needs it but Mary doesn't. What happens when Mary is the reviewer? Or Joe? Or Mark? – Andrew T Finnell Nov 23 '16 at 15:59
  • @jmoreno -- Not placing rules/guidelines about comments into the coding standards means programmers have to look at multiple sources -- and they don't know where to look until the review comes back saying ones comments are substandard. It is a mistake to think that everything in a coding standard must be automatable. Rules on meaningful names, for example, are not automatable. At least not yet. For example, `x`, `y`, and `z`, or `i`, `j`, and `k` can be quite meaningful names if one is implementing a scientific algorithm based on a journal paper that used exactly those names in its formulae. – David Hammen Nov 23 '16 at 16:39
  • @AndrewTFinnell: Re *The people picked for the review make the judgement call?* -- And your point is? It's a mistake, a big mistake IMHO, to think that every item in a coding standard must be automatable. Code is written once, read many times. This pertains many times so for life-critical / safety-critical software. Re *If new people don't think the same way or speak English?* That's a bit hyperbolic, particularly with regard to life-critical / safety-critical software. – David Hammen Nov 23 '16 at 16:46
  • @DavidHammen I dare suggest you haven't worked on "life-critical / safety-critical" software before? Without going into detail, I have experience on many projects that have been handed over to other organizations after a 2-year rebid occurs. Or, a Medical Device company which builds EMR software and brings in outside consultants, many of which English is not a native languge. It's not hyperbole, it's the way of life. To assume that the same people would be on a Mission Critical project forever is how those projects end up failing. Ensuring that code is indistinguishable from other code is key. – Andrew T Finnell Nov 23 '16 at 16:50
  • A Coding Standard, as is any Standard is a means of testing Quality of a given product. If the tests themselves are so subjective their interpretation changes depending on the person reading them, how can they ever be fruitful? There is a difference between holding key people accountable for producing a product that won't kill anyone creating a core group of Ivory Tower developers who pedantically take up days doing a single Code Review. It's to Review the Code not guess as to what the machine things is going to happen when it's ran. That's QAing via automated and manual testing. – Andrew T Finnell Nov 23 '16 at 16:55
  • How can one verify 'Don't write crappy comments.' Anyone here can write that 'Standard' in an infinite number of ways, but the meaning is the same. Or even better, 'Write good comments.' Again, this line item can be written thousands of ways, but ultimately they all have the same meaning. Define Good. If the chances of a developer getting code merged into the `develop` branch is predicated upon the 'Mood' of the person reviewing, it's a process smell. Humans applying a Standard may create errors. Who reviews the reviewer? There's a difference between teaching good design and semantics of code. – Andrew T Finnell Nov 23 '16 at 16:57
  • @AndrewTFinnell -- Working on safety-critical software is what I have done for a living for 35+ years. The most critical of them (e.g., "thow shalt not write software that kills astronauts") have almost uniformly had standards about comments/documentation. – David Hammen Nov 23 '16 at 17:03
  • @DavidHammen The same project and same code? When you transitioned from one project to another, assuming you did, did you find the comments to be the same caliber and quality as the previous projects? If not, why? If so, then I'd say your experiences differ than mine and you've been lucky. When you transitioned to the new project, who was 'right' and who was 'wrong?' Has your commenting style, density, wording changed in the last 35 years? Or has it remained the same? If it's changed then how would you create a Standard that would encompass the quality of your comments through 35 years? – Andrew T Finnell Nov 23 '16 at 17:06
  • @AndrewTFinnell -- Enough with the hyperbole. Obviously we disagree, and strongly, so it's rather pointless to continue this discussion, either here or in chat. – David Hammen Nov 23 '16 at 17:20
  • @DavidHammen Hyperbole - "exaggerated statements or claims not meant to be taken literally." You don't get to say if my statements are Hyperbole as I'm saying they are meant to be taken literally and are not exaggerated as I've seen it happen countless times. Perhaps you're trying to use a different word. And you didn't respond to my questions which leads me to believe you chose to attack me instead as my point was valid. This entire conversation is proof that you can't standardize comments. All these posts in this thread.. are comments. All my comments are relevant to the question. – Andrew T Finnell Nov 23 '16 at 17:21
9

Comment every line of code? No.

The purpose of the others rules you talk about is precisely to avoid that.

Comments to a readable code are at best redundant, and at worst will throw a reader off looking for the non-existent purpose of the comment.

If you comment whole code which is self-explanatory, you double the amount of reading with out giving any additional information. I am not sure if such redundancy would pay off. One can imagine a reviewer saying that 42 says "display_error" while the comment says "displays warning". But imagine a change in the code. You have two places to correct now. It becomes clear this style has copy-paste negatives.

I would say that optimally the code should not need any comments, other than documentation.

There are styles which go total opposite, if you have doubts about line it's either:

  1. The code is too complicated and should be refactored into a function with a name with a semantic meaning for the portion of code. Be it a complex if or a portion of an algorithm. (Or a clever LINQ one-liner)

  2. If it cannot be simplified, you don't know enough idioms of language.

(I personally am not a fanatic of strict no-comments rule, but I find it as a good initial mindset to go by.)

Given all this is it even possible to write good coding standards that capture this idea?

As for the process. Our standard gave quite much credit to the reviewer. Assuming they are not malicious this works good.

When he asks "What is variable o?", you rename it. If he asks what does this block do, either refactor or comment. If there was a debate whether something is clear or not, if the reviewer did not get it, then by definition it is not. On very rare occasions there was something like 2nd opinion from few friends.

On average, you should get a code understandable by the average programmer in the team. IMO it keeps off the redundant information, and ensures that the code is understandable by at least one other member of the team, which we found a good optimum.

Also, there were no absolutes. Though we were a small group. It's easy to find consensus in group of 5.

luk32
  • 400
  • 2
  • 7
  • 2
    Using code reviews to dictate what variables, classes and methods should be named is the best way to make people hate a company, business or methodology. Code reviews should not be used as a way of enforcing things that don't really matter. A Coding Standard must be checkable via a automated process. Variable name length, Circular Complexity, Law of Demeter, are all things which can be checked. If someone told me to rename a variable from 'i' to something like indexForThePreviousCollection, I would find another place to work. Variables > 3 chars is all checked before check-in automatically. – Andrew T Finnell Nov 20 '16 at 20:43
  • "If it cannot be simplified, you don't know enough idioms of language." - My guess is that there is nothing simple about processing the human genome. Sometimes something is complicated. It's not a good excuse to write poor code, however, sometimes it's just complicated. Good comments help explain why it was too complicated to make it more readable. Then there is the opposite where there's 'too much' simplicity. This is self evident when you have Java Developers that go full throttle with DI and create interfaces for every single class that is ever written thus making it impossible to follow. – Andrew T Finnell Nov 20 '16 at 20:49
  • 3
    @AndrewTFinnell I can write impossible to follow code in any programming paradigm you care to name. – candied_orange Nov 20 '16 at 21:25
  • 2
    @AndrewTFinnell Well, I will politely disagree. "*Using code reviews to dictate*", I don't know where you pulled that from. I would not like to work on the code with variables `i`, `j`, `k` all over the place after the original author quits. I don't understand your point on the "human" genome. I doubt there is a language where are single statement, or two are too hard to comprehend. As I've said, i've seen complex `if`s and `LINQ` one-liners. They can be either commented or refactored under a semantically meaningful name, I guess that is your "good comment." – luk32 Nov 20 '16 at 21:25
  • @AndrewTFinnell "*create interfaces for every single class*" is opposite of simplicity, it's indroducing a layer of abstraction and multiplies entities. I don't know which point of my answer inspired this statement. I disagree on "A Coding Standard must be checkable" it can be more then a set of logical rules. I would say no coding standard is fully checkable via a validator. Maybe pep8 is close. – luk32 Nov 20 '16 at 21:29
  • 3
    @luk32 I respect the right to disagree. Iteration variables are quite contextually obvious in my opinion. While I understand the sentiment of your statement, I believe there are too many practices which take that sentiment to the extreme. Do we really need code that reads: `for (int indexForCurrentDatabaseRow = 0; indexForCurrentDatabaseRow < currentUserRecordSetResults.length; indexForCurrentDatabaseRow = indexForCurrentDatabaseRow + 1)` versus `for (int i = 0; i < results.length; i++)` ? Perhaps it's a matter of preference and/or time spent looking at code. Overly verbose names smells to me. – Andrew T Finnell Nov 20 '16 at 22:14
  • @CandiedOrange I don't understand the meaning behind your statement. I think it's obvious we can all write code that isn't possible to follow. This Answer suggests that Code Reviews are about ensuring every developer can follow the code being checked in. I am disagreeing with that sentiment. I believe Code Reviews are NOT about ensuring Standards are followed but about ensuring mistakes aren't made. And naming a iteration variable `i` is not a mistake. More specifically it's possible to ensure code with a variable named `o` can never be checked in to begin with, PreReview, if required. – Andrew T Finnell Nov 20 '16 at 22:17
  • This has turned in a discussion and thus ought to be continued in a chat room if anyone is interested. These comments will likely be removed as it's not productive nor helpful to the original question. I've seen one too many teams who use Code Reviews to demonstrate how much power they have over others rather than ensuring the code is of quality. That assertion of power is disguised as 'quality control' when ultimately, the things being argued about have no bearing on the end result. We can see how this has already evoked emotional debate as I've attacked no one, yet I feel attacked. – Andrew T Finnell Nov 20 '16 at 22:20
  • I never stated nor suggested that 16 line long LINQ queries are of high quality. Nor did I suggest if statements which are logically difficult to follow are acceptable. Anything one would want to complain about in a Code Review semantically or logically can almost 100% be covered by LINT'ers. Complexity, branching logic, single return statements, Law of Demeter, variable naming, etc. All things which can be automated and never talked about in a code review. A Code Review is not a pissing contest.. yet.. it seems to become one quite often. – Andrew T Finnell Nov 20 '16 at 22:22
  • 2
    I don't think the names should be decided in code reviews. The code review should usually commence after the author thinks it's good enough, which also means it's properly commented, easy-to-understand and the variables have decent names. The thing is - if the reviewers think the code is too complicated or struggle with a variable's meaning, then the code is hard to understand. This is not debatable - evidently at least one developer - the reviewer - did not understand it. Whether or not it needs to be refactored(and how) or properly commented is a different matter. – Idan Arye Nov 20 '16 at 23:37
  • 1
    @AndrewTFinnell I think you got the main point: "*This Answer suggests that Code Reviews are about ensuring every developer can follow the code being checked in. I am disagreeing with that sentiment. I believe Code Reviews are NOT about ensuring Standards are followed but about ensuring mistakes aren't made.*" I believe the mistakes should be covered by testing, and CR should ensure the code is maintainable. I guess we can disagree on that. – luk32 Nov 21 '16 at 00:21
9

The question:

Given all this is it even possible to write good coding standards that capture this idea? Ones that will be relevant in a peer review but won't turn into a mindless checklist activity that produce notes no more helpful than: "You forgot to comment on line 42".

Comments should not seek to educate the reader on the language. A reader should be assumed to know the language better than the writer, but with much less context than the writer.

A reader of this code, from your example, should know that exit() will exit the application - thus making the comment redundant:

/* Exit the application */
exit();

Redundant comments are a violation of DRY, and changes to the code do not necessarily propagate to the comments, leaving comments that do not reflect what the code is actually doing.

However, comments that explain why you're doing something may be valuable - especially if you can't easily communicate the meaning in the code itself.

Python's PEP 8 (the style guide for Python in the CPython standard library) provides the following guideline for inline comments:

Use inline comments sparingly.

An inline comment is a comment on the same line as a statement. Inline comments should be separated by at least two spaces from the statement. They should start with a # and a single space.

Inline comments are unnecessary and in fact distracting if they state the obvious. Don't do this:

x = x + 1                 # Increment x

But sometimes, this is useful:

x = x + 1                 # Compensate for border

Given such an example, I personally prefer to go a step further and would try to eliminate this comment as well. For example, I might arrive at:

border_compensation = 1
compensated_x = x + border_compensation

Making your code self-documenting is not a new idea.

Back to the actual question:

Given all this is it even possible to write good coding standards that capture this idea?

I believe that the PEP 8 guidance and example demonstrates a good coding standard that captures this idea. So yes, I believe it is possible.

Aaron Hall
  • 5,895
  • 4
  • 25
  • 47
  • 1
    _"A reader should be assumed to know the language better than the writer"_ I have a hard time believing that's the proper attitude. I can see assuming they know _as much_, but every team is different, and every person on a team is different, so a blanket statement on this particular point doesn't seem wise. – Bryan Oakley Nov 21 '16 at 04:37
  • 4
    Without this guidance, you'll have junior programmers committing comments explaining trivial code to no one but themselves, and midlevel programmers using awkward solutions simply to make the code more accessible to junior programmers. This guidance eliminates redundancy, and ensures even a proficient programmer continues to improve their code. When I revisit code myself, I may have forgotten the context, but in general, I know what the language is doing even better than when I was first writing it. – Aaron Hall Nov 21 '16 at 04:50
4

I think when you see this kind of commenting, and I write this way myself occasionally, It's because the author is writing the spec out in the comments and then going through and implementing each comment.

If we are challenged to write a coding standard which produces code which is understandable in terms of its required functionality rather than its actually functionality (so that errors can be spotted) then are we really talking about how the specifications are defined, documented and linked to the final product?

edit ----

Just to clarify. I'm not getting into comments vs tdd vs unit tests which best. Or suggesting a comment per line is a good/bad

I'm just suggesting a reason you might see the style of commenting talked about in the example.

And from that questioning whether a coding standard about commenting would actualy be better written as a specification document requirement of some kind.

ie comments are for explaining the purpose the code, which is also what a spec is

Ewan
  • 70,664
  • 5
  • 76
  • 161
  • 6
    In 20 years I've never seen someome comment prototype their code this way then fill in the gaps with actual code. There is already an objective way to achieve what you're saying, or what I think you're saying. It's called Test Driven Development. The Unit Tests define the spec and whether the code adheres to that spec. All without comments. – Andrew T Finnell Nov 20 '16 at 20:04
  • 9
    While I don't think the code in the question is a good example of this, the practice of writing out a procedure in comments first, and then going back and filling in the code is specifically called out in "Code Complete" as a one possible practice to help produce readable code, @AndrewTFinnell. It's definitely not unheard of, even if it's outside your experience. – jscs Nov 20 '16 at 20:16
  • 6
    also predates tdd i believe – Ewan Nov 20 '16 at 20:28
  • 2
    Are you talking about the Pseudocode Programming Process? The purpose of that was to reduce comments. I don't ever remember that book stating the comments should be left in the code. And commenting to produce a conceptual procedure is meant to be high level, not low level. If one were to comment every line they intended to write they could have just written the lines. The idea is to prototype out the principals of the code not each line. The book does not suggest commenting every line. That is the focus of this question. And I could be wrong, but I believe that's in the Second Edition. – Andrew T Finnell Nov 20 '16 at 20:53
  • 1
    I think you misread the question. every line commented is an extreme example to avoid – Ewan Nov 20 '16 at 20:57
  • 2
    @JoshCaswell & Ewan, yes it *was* a technique many years ago and did indeed predate TDD. But, you were meant to delete the comments afterwards! And TDD has completely supplanted it as a sensible way of creating code that meets a spec. – David Arno Nov 20 '16 at 22:11
  • Yes, indeed, the Pseudocode Programming Process -- I'd forgotten the name. I don't have my copy of the book available, but I'm quite certain the pseudocode/comments are _not_ removed strictly. One of the advantages of the process is that useful comments are a consequence of it. – jscs Nov 21 '16 at 01:56
  • @AndrewTFinnell: _"In 20 years I've never seen someome comment prototype their code this way then fill in the gaps with actual code"_ I do it when the requirements are more than trivial - it helps with the thought process. However, the comments won't last more than twenty minutes and I certainly would not leave them in afterwards. They may (probably will) be replaced by meaningful explanatory comments for any non-trivial code lines. – Lightness Races in Orbit Nov 21 '16 at 12:55
  • 1
    An example of a useful comment is `// seek()ing here rather than ignore()ing due to libstdc++ bug #{NNNNNN} (which will {blah the blah} rather than {fonzle the fonz}): https://stackoverflow.com/q/40308080/560648` – Lightness Races in Orbit Nov 21 '16 at 12:58
  • An example of a useless comment is `// close the file stream` – Lightness Races in Orbit Nov 21 '16 at 12:59
  • 1
    its not useless if its followed by steam.open() – Ewan Nov 21 '16 at 13:06
  • @Ewan: Your compiler will catch the misspelling of `stream` and the missing argument to `open()` :) – Lightness Races in Orbit Nov 21 '16 at 20:07
  • its ok to run your code through autocorrect as a precompile step right?? – Ewan Nov 22 '16 at 00:17
  • @LightnessRacesinOrbit If Ewan's point was the misspelling of `stream` then I also think the compiler would catch it, but my initial interpretation was different: due to your example being about closing a stream and his reply being about opening one, I thought that his point was that the comment says "close the stream" and the code does the opposite. If it's the latter, I think that even such a comment would not be useless at all because it would allow to easily spot the error. Of course it might be the comment which is wrong, but now we got the chance to spot the potential error and check. – SantiBailors Nov 25 '16 at 11:09
  • i should have commented my comment!! – Ewan Nov 25 '16 at 11:16
4

Given all this is it even possible to write good coding standards that capture this idea? Ones that will be relevant in a peer review but won't turn into a mindless checklist activity that produce notes no more helpful than: "You forgot to comment on line 42".

This obviously goes above and beyond documentation - this touches how the code is structured, which algorithms are chosen, etc. It may even touch on requirements analysis and design.

My #1 rule is "Don't document the obvious." The #2 rule is "Write obvious code".

For safety-critical code (which I've never had to work on, thank God), this is a necessary but nowhere near sufficient first step. It may even have to come down to mandating what language features must be avoided (I've seen more than one standard mandating "Thou shalt not use scanf( "%s", input ); for any reason").

John Bode
  • 10,826
  • 1
  • 31
  • 43
3

IMHO it should do both. Commenting every line is silly, because you wind up with lines like

  i++;    /* Increment i */

with absolutely no clue as to why you'd want to increment i. Expand on that a bit, and you have the typical Doxygen style of comments, which produce a large amount of verbiage without supplying any idea of what the code is for, or how it works. (At least as far as I have ever seen: I admit that it might be possible to write useful documentation using a system like that, but I'm not holding my breath.)

OTOH, if you write a good comment block at the head of the function (or whatever grouping is logical), describing both what is to be accomplished, and how, if it's less than obvious, you have something that's useful. If you're me, you might even include LaTeX code for relevant equations, or references to papers, e.g. "This implements a WENO scheme for solving Hamilton-Jacobi equations, as discussed in..."

alexyorke
  • 232
  • 2
  • 9
jamesqf
  • 148
  • 1
  • 5
  • 2
    +1 for the doxygen comment: This is precisely my objection to including doxygen "documentation" in my code; 95% of the time it just repeats what's already obvious from the function signature. I, too, have not seen a single doxygen documentation that's worth anything, yet. Writing those comments wastes time three times: Once to write the comments, once to read them again, and once to debug problems created by stale comment content. – cmaster - reinstate monica Nov 20 '16 at 22:36
  • 1
    Aren't you contradicting yourself? I think we'd both agree that it's critical for a function to have a comment describing what it does from a black-box perspective - "these inputs go in, the function does this, and these outputs come out". It forms the contract between the person who wrote the function and the people who use the function. The purpose of Doxygen is simply to standardise the format of those comments in a way which can be parsed and made searchable/indexable. So why would you want a "good comment block" but **not** want it in a standard, human- and machine-readable format? – Graham Nov 23 '16 at 13:32
  • The use of a comment standard such as JSDoc, JavaDoc and Doxygen are meant for providing searchable and human readable documentation, as @Graham said. The line of code mentioned in this answer has nothing to do with Doxygen. I cannot even imagine a rule set being in place which would parse that comment and produce an entry in the outputted documentation. When you look at the Docs for JMI, the Java Standard Libraries, etc. they are all produced using a tool very similar to Doxygen. That's the purpose of it. Not what's said here – Andrew T Finnell Nov 23 '16 at 16:06
  • I'm not making this up - in order to meet a Canadian requirement, a company I worked at had the developers write a pre-processor that added comments EXACTLY like the OP's example. I think ours was "C ADD 1 TO I" or some such. It was FORTRAN, so loops were similarly commended "C LOOP FROM 1 TO 10". The code was LOADED with useless comments. I deleted all that crap as I maintained code. No one ever said anything to me. – Tony Ennis Nov 24 '16 at 15:27
3

The self-documenting code best practice is not a mainstream consensus - while it is widely acceptable that easy-to-understand code is better than cryptic code, not everyone agrees on whether or not complicated code must always be refactored or if it's OK to comment it.

But this debate is about pieces of code that are difficult to understand - and you are talking about commenting each and every line of code. Lines of code that are difficult to understand should be very rare - if most of your lines are complicated like this than you are overusing smart-ass one-liners and you should stop! Sure, a line's role may be a bit hard to understand sometimes, but that's it's role in the context of a bigger piece of code - what the line itself does is almost always straightforward.

So, you should not be commenting lines - you should be commenting pieces of code. Specific comments may be placed near the lines they refer to, but these comments are still referring to the bigger code. They do not describe what/why the line does - they describe what it does in that code and/or why is it needed in that code.

So, it is not lines that should be commented but bigger pieces of code. Should every piece of code be commented? No. Harold Abelson said that "Programs must be written for people to read, and only incidentally for machines to execute". But comments are only for people to read - the machine does not execute them. If your coding standards force writing redundant comments on every line/piece-of-code you are not just burdening the developer who needs to write them - you are also burdening the developers who will have to read them!

This is a problem, because when most of the comments you read are redundant, you stop paying attention to them(because you know they'll just be redundant junk), and then you'll miss out the important comments. So I say - write only the important comment, so that when someone see a comment in the code they'll know it's worth to read it in order to understand the code.

candied_orange
  • 102,279
  • 24
  • 197
  • 315
Idan Arye
  • 12,032
  • 31
  • 40
  • 1
    Miss out on the important comments: +1. The sentence that starts: "They do not describe" could use some restructuring to make it easier to follow. – candied_orange Nov 21 '16 at 02:17
2

It's time to bring back flow charts! (not really but hang on a minute)

The other day I found my old flowcharting template. I have only used it for homework and jokes (you gotta love a good silly flowchart). But even by this time most commercial programmers that were still using flow charts were generating them automatically from the code (which completely undermines the original purpose of the flowchart, which was to be an aide in writing better code and was quite useful in machine code. However with higher level languages the flowchart changed from being a crutch to a hoble. Why? the abstraction of the flowchart was the same as the code. The comments shown are a hobble in that they are at the same level of abstraction as the code. Good comments are at a different level of abstraction than the code. The easiest way to do this is to use comments for why while the code handles what. Other ways are to breakdown complicated steps to simpler alternates or to have manager level abstracts.

hildred
  • 739
  • 2
  • 7
  • 15
0

I'll answer the question as stated:

Given all this is it even possible to write good coding standards that capture this idea?

Yes. WYSIWYG. A good coding standard is literally "is is clear to the reader what the following code does and why".

In other words, my code review comments about code readability literally, 1-1, arise out of my mental state of

I, as a reader, (better yet, as a minimal standard, a mental model of a less-experienced but reasonable reader), would not understand the following code's purpose, or method of working

Generally, people are easy to get on board "this needs more readability" when they hear "I, as a senior developer who you hopefully respect as not being dumb, didn't at first read understand this code or why it's there or what it does, so it needs to be explained".

This shifts the discourse from "you didn't follow meaningless standard" to "your code has a specific readability deficiency leading to practical problem".

A second standard that needs to be made explicit is "2am emergency test".

Can your backup, who's not experienced with this code, figure out what the issue is with the code when debugging it during 2am production emergency bridge call, when you're on vacation in Chilean Andes with no cell phone?

This may sound subjective but is actually easy to practically measure: call up a random junior team member who would be expected to be the one debugging at night in production emergency, and ask them to explain how the particular un-commented code works and why it's there.

DVK
  • 3,576
  • 2
  • 24
  • 28
0

This answer covered most issues, however there is one important thing.

I'd like to capture an idea along the lines of: consider a comment for every line, sequence, statement, section, structure, function, method, class, package, component, ... of code.

There are tools to generate documentation from code, like for example doxygen. If you use such tools, then it makes sense to comment files, functions, classes, etc. Even if the function's name is self explanatory, I am still doing it for consistency, because people reading the documentation will be be grateful.

BЈовић
  • 13,981
  • 8
  • 61
  • 81
  • 1
    But the "documentation" generated by such tools is, at least in my experience, generally worth less than no documentation at all, because it doesn't provide the user with useful information (and causes them to waste time trying to extract the non-existent info), but fools the developers into thinking they're properly documented their code. – jamesqf Nov 22 '16 at 18:32
  • @jamesqf Yes, when half the functions are not documented, and other half badly. However doxygen can generate quite good documentation, assuming developers properly commented their functions, classes, etc. – BЈовић Nov 24 '16 at 08:45
-1

Many of the existing answers are well detailed, but I felt it important to answer:

...[comments] that will be relevant in a peer review but won't turn into a mindless checklist activity...

To me the only comments that could be a standard can be answered by asking what comments are noticed when they're missing. In other words: comments used by IDEs or documentation software.

That being said, this is subjective to what tools are used by the developers, and not all comments used by such software are as useful as each other.

Erdrik Ironrose
  • 4,806
  • 3
  • 13
  • 25