Proposal: Comment all the code

[Swaagie]

Something that might have popped up several times before. But it would be awesome to have all source code properly commented. Why are certain code decisions made and why not. This could benefit contribution and perhaps some minor issues that popped up in the past. Question is, is their broad support for this? What comment style should be used? Should code be stripped from comments before release to save space? Etc.

[filipecatraia]

I'm on board. I love documenting technical projects. (I start projects by documenting them.)

[bnoordhuis]

@Swaagie Can you post some examples of what you think commented code should look like?

The current comment style is reasonably spartan, preferring to let the code do the talking and only add explanations when it's not clear from context why the code is doing what it's doing (and that's how I think it should be.)

[bajtos]

The code should be ideally written in such way that it is easy to comprehend even without comments. As long as we don't take presence of a comments as an excuse for writing sloppy code, and keep the comments useful in the sense that they don't just rephrase the code in English language, then +1.

[3rd-Eden]

@bajtos I disagree with that. People learn how to code and contribute by reading code that's how we all started and that is something we should leave behind for others as well. What might be easy to comprehend for you can be voodoo for somebody else. And comments are ever an excuse for sloppy code. I would rather say code without comments is sloppy and unmaintainable code. Also reading the documentation and code side by side is painful.

[gkatsev]

There are also two types of comments that you need to consider. Function level comments that are more API documentation and in-line comments that explain the code associated with them. The function level/API docs are very important. The in-line comments hopefully are not needed in most cases but there are definitely cases where it would be useful, though, if you are writing short functions you can probably just have all the comments in the function level comment.

[bajtos]

@3rd-Eden

And comments are ever an excuse for sloppy code.

A counter example from the current node codebase (link):

// if we want the data now, just emit it.
if (state.flowing && state.length === 0 && !state.sync) {
  stream.emit('data', chunk);
  stream.read(0);
} else {
  // ...
}

Rewritten so that no comment is needed:

var dataWantedNow = state.flowing && state.length === 0 &&  !state.sync;
if (dataWantedNow) {
  stream.emit('data', chunk);
  stream.read(0);
} else {
  // ...
}

---

Few excerpts from Clean Code:

• The proper use of comments is to compensate for our failure to express ourself in code. Comments are always failures. We must have them because we cannot always figure out how to express ourselves without them, but their use is not a cause for celebration. So when you find yourself in a position where you need to write a comment, think it through and see whether there isn’t some way to turn the tables and express yourself in code.
• The older a comment is, and the farther away it is from the code it describes, the more likely it is to be just plain wrong. The reason is simple. Programmers can’t realistically maintain them.

• Comments Do Not Make Up for Bad Code! One of the more common motivations for writing comments is bad code. We write a module and we know it is confusing and disorganized. We know it’s a mess. So we say to ourselves, “Ooh, I’d better comment that!” No! You’d better clean it! Clear and expressive code with few comments is far superior to cluttered and complex code with lots of comments. Rather than spend your time writing the comments that explain the mess you’ve made, spend it cleaning that mess.

  // Bad:
  // Check to see if the employee is eligible for full benefits
    if ((employee.flags & HOURLY_FLAG) &&
    (employee.age > 65))
  
  // Good:
  if (employee.isEligibleForFullBenefits())

See also https://www.haskell.org/haskellwiki/Commenting

[filipecatraia]

+1 @bajtos for referencing Clean Code.

[imjacobclark]

+1 @bajtos and +1 for Clean Code

[imjacobclark]

Please see #83, I have made a start on improving code readability.

[aredridel]

A huge downside here comes in that this causes low-level divergence of the codebase from other branches; this is a huge place where merging and landing new features becomes more difficult.

[mhdatie]

I would also suggest, before any script is written, comment on what the script does and where it will be used. To list the other script file names that use the current script. If it's a lot of description, offer a link with a documentation for each script, with use cases as well.

I will be more than glad to help put with that.

[Swaagie]

In general, I'm with the argument that comments are never an excuse for sloppy code. But it's also a very general statement. Comments that have the scope of a single/few lines (like in the example @bajtos gave) should be avoided if and only if a descriptive variable can cover its semantic value. However, I disagree with the first notion that comments are always a failure. I get that it makes for a good strong point of view in the book). The reasoning behind creativity can only be expressed to certain extends in code.

@bnoordhuis I'm mostly a fan of the JSdoc comment style for function level comments. Although I do think markdown should and can also have a strong place in documentation. Mostly wondering if other people have strong preferences regarding comment styles. API documentation maintenance is done async so to speak, one of the things that comes to mind is the PR regarding options in TLS server creation, see https://github.com/joyent/node/pull/8553. Automatically generating the API would hugely benefit overall coverage and prevent differences across several modules. Examplary could be something like

/**
 * Brief but specific explanation of the function at hand. 
 *
 * @param {String} host Descriptive explanation of the variable if required.
 * @param {Object} cert Descriptive explanation of the variable if required.
 * @api private
 */
exports.checkServerIdentity = function checkServerIdentity(host, cert) {

or including markdown

//
// Brief but specific explanation of the function at hand. 
//
// # function checkServerIdentity
// ### @host {String} Descriptive explanation of the variable if required.
// ### @cert {Object} Descriptive explanation of the variable if required.
//  
// **@api public**
//
exports.checkServerIdentity = function checkServerIdentity(host, cert) {

Added bonus of using markdown is that the API documentation can be styled easily and important statements or arugments can be highlighted. It does make it a little bit more messy though

@bajtos thanks for linking that, I wasn't aware of that reference. Will definitely read up on that.

@aredridel I agree, documentation could create a huge influx in merge failures. But I don't think we should be afraid to combat those, as other branches are probably more than willingly to include good documentation.

@MohamadAtieh if I understand you correctly you'd like a network graph of modules, basically describing the relationship between them? If so, we could also automatically generate those (ast parser comes to mind), but I could be helpful.

[aredridel]

I'm no lover of jsdoc: I think it's inflexible, and highlights the least useful parts of the documentation. I think we have a lot better result with documentation in prose form in other files, doubly so because the signatures of javascript functions are verbose to describe in full. param {Object} is not useful at all. What keys are expected? Is the namespace open to extension or closed? To whom? how stable is this interface? What about variadic signatures, where arguments is inspected and the meaning figured out?

Trying to shoehorn them into a syntax-heavy, hard-to-parse, mixed-with-javascript format does us a disservice in trying to write well and completely.

[mhdatie]

@Swaagie Yeah a graph of modules/helpers and their relationships. Diagrams of how and what methods are being called, params and stuff like that. A short and clear quick reference of the structure.

[tellnes]

+1 what @aredridel sad about jsdoc. Most of the information in @Swaagie's example is self explanatory by reading the code.

[brandonscott]

I agree with @Swaagie's comment on how useful it is for automatic generation of documentation which reduces duplication by not having different comments in code and multiple separate technical documents.

For new developers, even well expressed code can be challenging to get around and I think we should offer them a chance to use the project by providing a good starting block.

If we don't want to add chunks of comments to every method, why not use a static code analyzer to pull in all the methods and link them to markdown written in one consolidated parsable document. This could be added to an automated build process and seems to be a compromise around all of the above comments.

[rlidwka]

I agree with @Swaagie's comment on how useful it is for automatic generation of documentation

I want to point out, that it is "automatic generation of useless documentation".

Copying my answer from earlier threads:

The issue with JSDoc is that people try follow strict format, and basically end up with writing documentation for computers forgetting who will be reading it.

For example, here is a typical readme:

## Markdown.parse(input[, options])

Parses markdown language. Options object may contain these options:
 - gfm -- enable github-flavored markdown features
 - sanitize -- don't allow user-specified html tags in the output

And here is JSDoc most of projects would use:

/**
 *  Parses markdown.
 *  @param {string} input The string to parse.
 *  @param {Object} options Options.
 *  @returns {string} Parsed string.
 */
function parse(input, options) {

Those are very realistic examples, and it took me about the same amount of time to write both. But in first example most effort is spent on trying to help a user, and in the second one it's spent on trying to conform to the format and to avoid travis build failing.

@param {Object} options Options. is not even a joke, I saw hundreds of cases when JSDoc is enforced, so people write this just to conform to the standard. And everyone thinks that's okay, even though they have a documentation that is essentially useless.

[feross]

@rlidwka I'm not a huge fan of JSDoc, but to be fair, it is possible write useful comments in that style. For example:

/**
 * Create a torrent.
 * @param  {string|File|FileList|Blob|Buffer|Stream|Array.<File|Blob|Buffer|Steam>} input
 * @param  {Object} opts
 * @param  {string=} opts.name
 * @param  {Date=} opts.creationDate
 * @param  {string=} opts.comment
 * @param  {string=} opts.createdBy
 * @param  {boolean|number=} opts.private
 * @param  {number=} opts.pieceLength
 * @param  {Array.<Array.<string>>=} opts.announceList
 * @param  {Array.<string>=} opts.urlList
 * @param  {function} cb
 * @return {Buffer} buffer of .torrent file data
 */
function createTorrent (input, opts, cb) {
  // ...
}

from https://github.com/feross/create-torrent/blob/master/index.js#L17

[rlidwka]

@feross,

I don't doubt that it's possible to write useful documentation using JSDoc. But I doubt that many people will do that.

It's like brainfuck in a sense. It is possible to write useful programs in that language (because it's turing-complete), but I doubt many people will do that.

Also, if we're choosing between this:

 * @param  {Object} opts
 * @param  {string=} opts.name
 * @param  {Date=} opts.creationDate
 * @param  {string=} opts.comment
 * @param  {string=} opts.createdBy
 * @param  {boolean|number=} opts.private
 * @param  {number=} opts.pieceLength
 * @param  {Array.<Array.<string>>=} opts.announceList
 * @param  {Array.<string>=} opts.urlList

And this:

 - options object, consists of the following parameters:
    - name (string)
    - creationDate (Date)
    - comment (string)
    - createdBy (string)
    - private (boolean | number)
    - pieceLength (number)
    - announceList (array[array[string]])
    - urlList (array[string])

I'd pick the second one every time.

[bajtos]

@caineio

1. yes
2. other (all parts)
3. v0.10, v0.12, v1.0.0 (all versions)

[vtambourine]

@rlidwka,

We can choose simple and laconic JSDoc notation, but let we all agree that in-code documentation is quite useful for projects, just because it makes future contributors dive faster into source.

[jakubkeller]

+1

On Thu Dec 18 2014 at 11:19:58 AM Benjamin Tambourine < notifications@github.com> wrote:

@rlidwka https://github.com/rlidwka,

We can choose simple and laconic JSDoc notation, but let we all agree that in-code documentation is quite useful for projects, just because it makes future contributors dive faster into source.

— Reply to this email directly or view it on GitHub https://github.com/iojs/io.js/issues/60#issuecomment-67510964.

[bnoordhuis]

I personally see little value in jsdoc-style comments for reasons already outlined by others. What could work, perhaps, is GHC-style notes as described here.

In my limited experience hacking GHC core, notes work pretty well for conveying the big picture without repeating that information everywhere. Unlike architecture documentation, it's inline and less likely to go stale.

[chrisdickinson]

Closing this due to a lack of activity + lack of an actionable outcome. I don't think we're likely to entertain large-scale cleanups (simply because it's very hard to adequately review diffs of that size!) If you see a section of code that you feel can be cleaned up, feel free to do so and make a PR – keep in mind that these sorts of changes will not be high priority for reviewers, though.

In general, I'd encourage leaving it to the author of the code and their reviewers to decide how much and what kind of comments are necessary to understand a given piece of code instead of mandating a certain amount of comments.

That said, if you are interested in improving io.js' documentation, please ping me (via email, twitter, carrier pigeon, etc.)