Should we document the private classes and functions?

[sequba]

Context

@budnix Suggested that we should add jsdocs (a code comment containing documentation information) to every class and every method in the project (even private ones). Some of the arguments for that:

1. increased readability (being able to understand the purpose and technicalities without reading the implementation of the function)
2. lowering the entry threshold for new contributors
3. better visual organization of the code (grey areas separating code entities in the IDE)

I understand this point of view, but in my opinion, arguments 1 and 2 apply only to more complex methods. Most code entities are simple enough that the function signature (the name and the type) explains them sufficiently. Also, we need to take into account the cost of creating and maintaining the jsdocs for all the classes and methods in the project.

Undoubtedly there are code entities that should be documented:

• all methods in our public API
• all methods containing complex calculations or logic which is not explained by the function signature
• all code that might be confusing to the reader for any reason and the developer is unable to make it clear

To summarize my position, I value common sense over strict rules and I prefer the decision of whether or not to add a jsdoc to be left to the developer.

Example of a method that requires jsdoc:

/**
 * Searches for the searchKey in a sorted array.
 * Param orderingDirection must be set to either 'asc' or 'desc' to indicate the ordering direction of the array.
 *
 * Semantics:
 * - If orderingDirection === 'asc', searches for the lower bound for the searchKey value.
 * - If orderingDirection === 'desc', searches for the upper bound for the searchKey value.
 * - If the array contains duplicates, returns the last matching value.
 * - If no value in the range satisfies the above, returns -1.
 */
export function findLastOccurrenceInOrderedArray(searchKey: RawNoErrorScalarValue, array: RawInterpreterValue[], orderingDirection: 'asc' | 'desc' = 'asc'): number {
  //...
}

Example of a method for which the jsdoc is redundant:

  /**
   * Returns the array of arrays with all cell values from the sheet specified by the sheetId 
   */
  public getSheetValues(sheetId: number): CellValue[][] {
    //...
  }

CC: @krzysztofspilka @warpech

Links:

The discussion started in a private slack thread: https://handsoncode.slack.com/archives/C044LF2LF71/p1666078448176849 (accessible by @budnix @krzysztofspilka and @sequba)

[sequba]

@krzysztofspilka As far as I can tell we have:

~420 documented code entities
1904 undocumented code entities

--
Technicalities:

• Analysis performed on develop branch (3bdb15f).
• Directory: src/.
• Entities with jsdoc found using WebStorm find feature by executing regex: \n\s*\*/\n[^\n]+ (the result might not be 100% accurate but it's very close).
• Entities without jsdoc found by eslint with eslint-plugin-jsdoc@v36.1.1 configured with a single rule:

  rules: {
    "jsdoc/require-jsdoc": ['error', {
      require: {
        ArrowFunctionExpression: true,
        ClassDeclaration: true,
        ClassExpression: true,
        FunctionDeclaration: true,
        FunctionExpression: true,
        MethodDefinition: true,
      }
    }],
  }

[sequba]

We decided to start documenting private code.

Related:

• Setup eslint to report undocumented code #1085
• API reference should contain only the public API #1086