3

I've been reading Robert Martin's book "Clean Code". One of his core tenants is to remove unnecessary comments and strive to create meaningful variable/method names that are self documenting.

Some of my colleagues disagree with this approach arguing that it is impractical for large software projects. They cite the example of jQuery's codebase which is littered with comments, unclear variable names like fn and sometimes even commented out code, e.g. jquery/deferred.js

Their conclusion is that open source code can't really follow clean coding principles because people want line by line explanations for why a particular approach was used and so liberal comments are a necessity. They also argue that the long descriptive method names advocated by Uncle Bob are harder to read than a short name with a descriptive function comment.

Do you think this conclusion is true? If not, do you have examples of vendor codebases that follow Clean Code principles faithfully?

Codebases I've examined

jQuery:

see above

Angular JS:

Example: Angular.js
- lots of comments that sometimes seem disconnected with context
- very long methods not following the extract method pattern

React JS

Example: ReactComponent.js

- Not much vertical spacing to separate code blocks
- relatively long methods not following extract method pattern
+ comments relatively rare and generally used to explain obtuse cases that would not be obvious

jeznag
  • 147
  • 4
  • 5
    Just a note, fn is NOT unclear. Whilst it's an abbreviation, it's very common is JS code to name a function fn. – Silviu Burcea Sep 16 '15 at 06:18
  • 1
    All coding guidelines are not equal. One should examine one rule at a time to determine whether it is appropriate for the situation. The different elements of coding style do interact with each other, but it wouldn't make a good question to debate whether coding guidelines as a whole need to be enforced or thrown away entirely. – rwong Sep 16 '15 at 06:25
  • Separation of comments from source code requires [advanced tool support](https://github.com/blog/622-inline-commit-notes), something that is not universally achievable because people will want to use their own favorite tools for editing source code. – rwong Sep 16 '15 at 06:28
  • 1
    Large open source projects would be in the hands of maintainers (sometimes the original inventors and creators), and also would need to foster consensus among its users. [This question](http://programmers.stackexchange.com/questions/226440/is-it-okay-to-make-coding-style-changes-on-an-open-source-project-that-doesnt-f) explores the options one can make when approaching a project which does not follow the prevailing coding guidelines for the particular programming language. – rwong Sep 16 '15 at 06:32
  • 1
    Just because some open source projects do not follow clean coding principles, doesn't mean they can't. The maintainers of those projects just choose not to. So no, the conclusion is not true. – David Arno Sep 16 '15 at 10:36
  • I've never understood the "self-documenting names" rule. For example, in mathematics we choose sufficiently recognizable names such as `cos(x)`, which is accompanied by a comment (e.g. in your math book in this case) to explain what `cos` and `x` mathematical concepts are. We wouldn't be helping anyone if we had instead written `ComputeCosine(angleAmountInRadians)` or something. – Brandin Sep 16 '15 at 13:36
  • 2
    @Brandin ... and as a result, attempting to dip into some mathematics without context is an absolute nightmare. Following structured lecture notes as a student, or reading a whole book on a topic - easy enough. Diving in and trying to understand a proof of a theorem without knowledge of the surrounding field? You won't even be able to understand the syntax, and it won't be syntax you can easily Google for the meaning of either. This is a tolerable state of affairs for mathematicians, but if programmers had the same culture, it would basically make maintenance programming impossible. – Mark Amery Sep 16 '15 at 16:08
  • 1
    @Brandin 1) Math has a few millennia of tradition with its naming schemes, and has been mostly hand written without fancy autocompletion. 2) Yes, `ComputeCosine` is a terrible name, except perhaps in case you wanted to differentiate between a function that computed cosine and one that did a lookup. 3) `angleAmountInRadians` should ideally be `angle` and be of type `Angle`, from which radians or degrees are easily extractable. For languages with poor types, `radians` as a `float` might suffice. So `cosine(angle)` really that bad compared to `cos(x)`? – 8bittree Sep 16 '15 at 16:13
  • @8bittree I guess `cosine(angle)` wouldn't be that bad. But the point is that `cos(x)` is *just fine*. Tons of computer function names are traditional as well. Should we go back to the 70s and rename `printf` to `FormattedPrint`? Sometimes you've just got to read the manual to figure out what the symbols mean, and that's just fine. – Brandin Sep 16 '15 at 16:19
  • 1
    @Brandin `printf` does at least have `print` in the name. Admittedly, the `f` is a bit ambiguous. It is more descriptive than `cout <<`. But, `printf` and `cout` do have the advantage of being just about the first thing to learn in a language, and they are *consistent*. `printf` in one program is likely to be the same as `printf` in another because it's part of the language's library. I'll grant you that `cos(x)` is similarly consistent (or more so). `doThing(a)`, however, is meaningless and likely to be different in other places, even one project. Compare that to `verifyPassword(password)`. – 8bittree Sep 16 '15 at 16:59
  • @8bittree My point is you're going to have to document the stuff *anyway*, so ultimately it doesn't matter at all whether you write `verifyPassword(password)` in the documentation or `verifyPassword(p)`. `p` is just as good a name as `pw` or `password` or `thePassword`. The idea to remove documentation by making symbols "sufficiently clear" is a fool's errand. – Brandin Sep 16 '15 at 18:59
  • 1
    @Brandin `verifyPassword()` is self documenting, assuming that it does, in fact, verify the password. `vp() // verifies the password` is not self documenting. No, I do not think documentation should be removed entirely. A serial number validating function should be accompanied by documentation explaining what constitutes a valid serial number, but the function name should make it clear what it does. Shamus Young has [an article](http://www.shamusyoung.com/twentysidedtale/?p=9892) illustrating some of the benefits of both great comments (including ASCII art) and self-documenting code. – 8bittree Sep 16 '15 at 19:56

5 Answers5

11

Just to be clear, Bob did not invent the concept of clean code. He just wrote a book with that title containing some very sound advice and some more dubious advice and opinions. I will bet all maintainers of large open source projects believe in clean code, but they do not necessarily adhere to all the principles in that specific book.

That said, I think you are misstating the opinion of Bob regarding comments. Bob advocates removing unnecessary comments. Unnecessary comments are comments which state what is already obvious from the code, like the classical:

// increment i by one
i++;

Or comments that only compensate for bad variable/function naming or needlessly convoluted code.

But "line by line explanations for why a particular approach was used" is definitely not unnecessary, since code - however clean it is written - typically does not in itself explain why a certain approach is used.

Looking at the source for Angular.js for example, you find two kinds of comments.

1) API documentation comments like:

/**
 * @ngdoc function
 * @name angular.isObject
 * @module ng
 * @kind function
 *
 * @description
 * Determines if a reference is an `Object`. Unlike `typeof` in JavaScript, `null`s are not
 * considered to be objects. Note that JavaScript arrays are objects.
 *
 * @param {*} value Reference to check.
 * @returns {boolean} True if `value` is an `Object` but not `null`.
 */

2) Inline explantation comments like: 

// Need to check if hasOwnProperty exists,
// as on IE8 the result of querySelectorAll is an object without a hasOwnProperty function

Both kind of comments are absolutely an indication of clean, high quality code. The API documentation explains the workings of the library for the user, and the inline comments explain why the code is written as it is, i.e. it provides information which is not present in the code itself.

JacquesB
  • 57,310
  • 21
  • 127
  • 176
  • 6
    It's not just that you should remove already-unnecessary comments, it's about whether the code could be changed to *make* the comments unnecessary. People often seem to either totally ignore this or be dogmatic about it ("no comments!"), but the sensible thing is: When you see a comment think "Is there a change I can make to the code would would render this unnecessary? Would that change be a positive one?" If yes to both, do it. If no, keep the comment. – Ben Aaronson Sep 16 '15 at 12:34
  • @BenAaronson yep. Mundane example: variable name changes can often remove the need for a comment explaining what the variable contains. – Mark Amery Sep 16 '15 at 16:12
8

... people want line by line explanations for why a particular approach was used ...

Can you can read and understand what the code does? You don't need comments.

Do you understand why the code does what it does? You don't need comments.

Otherwise, comments are probably a Good Idea.

Now: wind the clock forward "some time" (years? months? weeks, even?)

Can you still understand what the code does? Why it does it?
This is where good commenting pays for itself.

You're not going to remember any given piece of code a year after you write it, but your comments should help you get "back into it" faster.

Phill W.
  • 11,891
  • 4
  • 21
  • 36
  • If the first two questions are answered with `No`, you should try to clearify your code first. – maklemenz Sep 16 '15 at 12:05
  • 2
    Code is intended to express a process to a machine. No matter how much you clarify it, there are times when the *reasoning* behind a process is not clear. – asthasr Sep 16 '15 at 12:13
  • If you simply replace the words "good commenting" in your answer with "well-written code", and "your comments" with "your code", you will have an answer arguing perfectly for the "clean code" perspective. – Eric King Sep 16 '15 at 13:07
  • @syrion Some would disagree with that assertion. "First, we want to establish the idea that a computer language is not just a way of getting a computer to perform operations but rather that it is a novel formal medium for expressing ideas about methodology. Thus, programs *must be written for people to read, and only incidentally for machines to execute*." [Structure and Interpretation of Computer Programs](https://mitpress.mit.edu/sicp/front/node3.html) – Eric King Sep 16 '15 at 16:24
  • @EricKing I am familiar with that idea. It also ignores one of the most important aspects of programming: why is one approach chosen over another when both may be *functionally* equivalent? [This performance improvement](http://jvns.ca/blog/2015/09/10/a-millisecond-isnt-fast-and-how-we-fixed-it/) is a good example. If there is an important but non-obvious reason that something should happen in a particular way, a comment is the only real way to explain why. – asthasr Sep 16 '15 at 17:38
  • 1
    @syrion Yep, and nothing in *Clean Code* indicates otherwise. But I was talking about the assertion that *we write code for machines to read*, and therefore it's ok when we have obtuse code that only a machine would understand, as long as we have a comment explaining it. I and others assert that the code should be written as to be readily understandable by *humans* first, and only if that is impossible or impractical should we fall back to comments as our explanatory mechanism. – Eric King Sep 16 '15 at 17:46
  • @EricKing There isn't really a difference there. Even though we write code to be human readable, you are still describing process rather than reasoning (unless, perhaps, you are writing declarative languages). – asthasr Sep 16 '15 at 17:50
0

The key portion of his position is strive towards, not achieve. It's a question to if you want to spend time polishing variable names and documentation or you got it working and just pushed the code. I guess people would rather write some new fun feature than clean up dead code.

About the specific source you linked. From a quick look at angular the majority of the comments were public API documentation which he specifically called out as good(page 59 of my copy). There also seem to be a bunch of them that were referencing specific changes and linking back to the related issue like

//Encode angle brackets to prevent input from being sanitized to empty string #8683

The deferred.js example does seem to have a fair bit of issues though with commented out code and a couple of places where it looks like the comment and the implementation don't match. It too has a number of comments that reference either the promises spec which are valuable.

It's all open source, if you think you could improve it, get involved and send them a pull request. The next guy will thank you.

Sign
  • 2,643
  • 19
  • 22
-1

Not always big companies obey rules, even though the rules are for the benefit of people.

Similar situations rise back in the times when Internet Explorer (Microsoft) do not follow all the rules of World Wide Web Consortium. Some old versions of IE have some implementations for JavaScript and CSS which are working differently than the W3Consortium definitions, which cause designers to deal with Browser-based exceptions.

So being a big company or being a product of a big company or being a big open-source project would not mean it will obey the standards.

Standards are for making our life easier. Same is true for clean code rules, though some people find some rules pointless or unnecessary. But that do not mean those rules are pointless.

Think about the situations below:

  • You are working with 10+ developers on a project. You use version controlling and every developer have to work on many files. You have to use a class/module written by 5 different developer. The class/module is a quite long script and you have to figure out how it works so you can use it correctly. Would you prefer a well documented cod that cost you less time to understand or a long code with proper naming?
  • In your project, you need a module that will do a single job for you. You found an open-source project that is great. But unfortunately it is 10.000+ lines long and you have to figure out how you should use it. It have long explanatory variable names but (since it was not you who have written the code) no docstring.Would you prefer reading the whole project?
  • You need a module and get an open-source package which have a great on-site documentation. Then you start using it and a few weeks later boooom! The open-source project have a bug that effects your situation. You open a ticket but unfortunately, either the project is no longer supported and developed or the team is so busy that they scheduled your bug fix to a laterer unknown date. You open the source code. It have finely named variables and functions, but have no comments. So you have no idea where to start searching.

So, having self explanatory names is good. Having comments are good too but having both is better. Some big projects may refuse to obey clean code rules but that do not make those rules useless. Even Microsoft and other big companies sometimes do not obey the rules and that would make you nervous when you need those rules had applied. On that time, you understand how important those rules are.

Mp0int
  • 429
  • 3
  • 12
  • 1
    I hate when someone is down voting without giving a reason... you've spent decent amount of time writing it, someone should at least give you a hint what's wrong with it. – Karol Sep 16 '15 at 23:42
-1

Commenting is good - writing self-documenting code is just better. Usually, commenting takes less time than writing self documenting code. People working on open source project, even some of the big ones, are likely not to get paid for that, and contribute in their free time, so some of them might take the easier way, instead of keeping high standards like they would in their day job. Sometimes it's just pragmatic to minimize efforts even at cost of quality.

Borsunho
  • 99
  • 1