144

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

  • 21
    Comments are not for extended discussion; this conversation has been moved to chat. – maple_shaft Nov 23 '16 at 17:04
  • 32
    “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 - rtfm-sarl.ch/articles/hide-comments.html ! – Michael Durrant Nov 25 '16 at 21:00
  • 1
    Also toggling - stackoverflow.com/a/11842371/631619 – Michael Durrant Nov 25 '16 at 21:01

17 Answers 17

171

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.

  • 46
    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! :( – user223083 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 7: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 G - justice for Monica Nov 21 '16 at 10:24
  • 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 1:05
151

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

  • 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 0:34
  • 1
    What is the other hard problem? – David says Reinstate Monica Nov 21 '16 at 16:13
  • 20
    @DavidGrinberg cache invalidation and off-by-one errors. – Eric King Nov 21 '16 at 17:07
  • 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 2:10
23

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.

  • 63
    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
  • 10
    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
19

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.

  • 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 2: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
  • 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
14

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.

  • 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
  • 1
    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 2: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 5:05
  • 1
    @AndrewTFinnell I mean that if you sometimes get a division by zero error, adding "if(divisor == 0) return <something>; // 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
12

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.

  • 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 6: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 8: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.

  • 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. – Trilarion 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 2: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
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.

  • 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
  • 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 ifs 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
  • 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
  • 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
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.

  • 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 4: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 4: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

  • 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. – Josh Caswell 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
  • 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
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").

  • Obvious is in the eye of the beholder. And changes depending on the deadline. – Tony Ennis Nov 24 '16 at 15:24
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.

  • 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 2:17
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..."

  • 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 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
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.

  • I am sorry to inform you that flowcharts never went away. – Ant P Nov 20 '16 at 23:31
  • Good comments are at a different level of abstraction than the code. Hear hear! – candied_orange Nov 21 '16 at 2:45
  • 1
    While this might be decent advice, it doesn't seem to address the question that was asked, except for perhaps a single sentence. – Bryan Oakley Nov 21 '16 at 4:33
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.

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.

  • 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 8: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.

Not the answer you're looking for? Browse other questions tagged or ask your own question.