Discussion on: Does extensive documentation do more harm than good?

View post

As an experienced Developer and Technical Lead for a development team I dislike so many comments wihtin the source code. There are a number of reasons for this

Once changes are made to the code the comments are out of date unless updated as well, which means we have to update two places (DRY)
Comments that are out of sync with the code are a negative when trying to make changes/bug fixes. They make it difficult to understand what the code is doing, because it doesnt match the comment.
A lot of comments in source code makes it difficult to read the source code and build a mental model of the code itself. As Doctor Bob says in Clean Code - the use of comments is always because we could not express ourselves in code well enough

So I strongly dislike comments in code files

Documentation is another story. Good documentation about the library or functionality is a god send! I always support making good documentation that explains the FUNCTIONAL purspose of code/library. It should not include a technical breakdown as we end up with the same problem as with comments in code
So documentation should be

Explains Functional breakdown
Meaningful to the reader
The only exception here is that for libraries examples on using the library and examples on calling functions are often very helpful

I'm still working on improving my ability to write meaningful documentation for my work, especially for libraries I create.

Vandolf Estrellado • Jan 3 '22

Wow, thanks for the thorough feedback. I pretty much agree with you on all your points! I try to only write documentation that I feel is required. 100% of the time that is documentation about the input, output, and possible side effects. I only question if I overdo it sometimes =)

William Cairns • Jan 3 '22

If the function definition are the inputs - and the name of the function should make it clear what it returns :) So adding those is quite possibly more than is needed

If I have a function called
getUserDetails(id)

It should be clear to you that you send in a user id and will get a details object back. It doesnt need comments. As the coder using this function you should not need to know where the userDetails are coming from, if you did then the name should be improved. I use id to represent the primary key, this has become a norm within the team, so everyone would know what the id field should contain :)

Vandolf Estrellado • Jan 3 '22

Again, I 100% agree! For functions that are self-explanatory, there is no need for documentation =)

There is a special case where I disobey this rule. That is when it seems like it is self-explanatory but it actually isn't. When wrapping system-level databases that has its own set of rules that you have no control over, I write extensive documentation to ensure that consumers of the property/function are aware of those rules.

For example,

 /**
     * The name that should be used to display the (raw) contact. This is the unstructured component
     * of the name should be consistent with its structured representation.
     *
     * The [displayName] is the unstructured representation of the name. It is made up of structured
     * components; [prefix], [givenName], [middleName], [familyName], and [suffix].
     *
     * ## Insert/update operations
     *
     * You have three different options when inserting/updating a [NameEntity],
     *
     * 1. If the [displayName] is null and there are non-null structured components provided (e.g.
     *   [givenName] and [familyName]), the Contacts Provider will automatically set the
     *   [displayName] by combining the structured components.
     *
     * 2. If the [displayName] is not null and all structured components are null, the Contacts
     *   Provider automatically (to the best of its ability) sets the values for all the structured
     *   components.
     *
     * 3. If the [displayName] and structured components are not null, the Contacts Provider does
     *   nothing automatically.
     *
     * #### Important things to know about
     *
     * If your app only allows users to update the structured components and not the combined
     * [displayName], you should set the [displayName] to null when performing an update. This means
     * **option 1 is for you**. Otherwise, if you are trying to set all structured components to
     * null but you leave the [displayName] not null, the Contacts Provider will automatically set
     * the value(s) of the structured components to a derived value from the [displayName]. In
     * effect, your app would seemingly not allow users to clear the name.
     *
     * If your app only allows users to update the [displayName] and not the structured components,
     * you should set the structured components to null when performing an update. This means
     * **option 2 is for you**. Otherwise, if you are trying to set the [displayName] to null but
     * you leave the structured components not null, the Contacts Provider will automatically set
     * the value of the [displayName] to a combined value from the structured components. In effect,
     * your app would seemingly not allow users to clear the [displayName].
     *
     * If you want to manually update both the [displayName] and structured components with your own
     * custom algorithm, you may do so at your own discretion =)
     *
     * ## [ContactEntity.displayNamePrimary] vs [Name.displayName]
     *
     * The [ContactEntity.displayNamePrimary] may be different than [Name.displayName]. If a [Name]
     * in the Data table is not provided, then other kinds of data will be used as the Contact's
     * display name. For example, if an [Email] is provided but no [Name] then the display name will
     * be the email. When a [Name] is inserted, the Contacts Provider automatically updates the
     * [ContactEntity.displayNamePrimary].
     *
     * If data rows suitable to be a [ContactEntity.displayNamePrimary] are not available, it will
     * be null.
     *
     * Data suitable to be a Contacts row display name are;
     *
     * - [Organization]
     * - [Email]
     * - [Name]
     * - [Nickname]
     * - [Phone]
     *
     * The [ContactEntity.displayNamePrimary] is automatically resolved by the Contacts Provider. It
     * may not be manually modified.
     */
    val displayName: String?

Without the documentation above, I feel like users of this property will make mistakes and create bugs. Actually, the reason why I have so much documentation on this one property is because I encountered a head-scratching bug with it.

In any case, you are correct. There is no need for documentation on self-explanatory code.