Live data from Hacker News

Self-Documenting Code

lackofimagination.org

1–10 of 131 posts

Re: Self-Documenting Code

#3
I agree with most of the article but want to nitpick this last part:

> I’m not a fan of TypeScript, but I appreciate its ability to perform static type checks. Fortunately, there’s a way to add static type checking to JavaScript using only JSDoc comments.

If you're writing JSDoc comments, then you're not writing what the author considers to be "self-documenting code."

I wish the author had explained why they are not a fan of TypeScript. Compile time type-safety aside, as the author acknowledges by implication adding type specificity negates the usefulness of JSDoc comments for this particular situation.

I'm personally a big proponent of "self documenting code" but I usually word it as "code that serves as its own documentation because it reads clearly."

Beyond "I would personally use TypeScript to solve that problem", my case for why ALL comments are a code smell (including JSDoc comments, and in my personal opinion) is:

- They are part of your code, and so they need to be maintained just like the rest of your code

- But ... they are "psychologically invisible" to the majority of developers. Our IDEs tend to gray them out by default etc. No one reads them.

- Therefore, comments can become out of sync with the code quite easily.

- Comments are often used to explain what confusing code does. Which means that instead of fixing the code to add clarity, they do nothing but shine a spotlight on the fact that the code is confusing.

- In doing the above, they make messy code even messier.

I am slightly amenable to the idea that a good comment is one that explains WHY weird code is weird. Even then, if you have the luxury of writing greenfield code, and you still need to do something un-intuitive or weird for really good reasons ... you can still write code that explains the "why" through good naming and separation of concerns.

The only time that I would concede that a code comment was the best way to go about things in context is when you're working with a very large, legacy commercial code-base that is plagued by existing tech debt and you have no good options other than to do your weird thing inline and explain why for logistical and business reasons. Maybe the refactor would be way too risky and the system is not under test, the business has its objectives and there's just no way that you can reasonably refactor in time etc. This happens... but professional developers should ideally treat incremental refactoring as a routine part of the development lifecycle so that this situation is as unlikely as possible to arise in the future.

Re: Self-Documenting Code

#4
I've been developing for a very long time and I'm neither on the side of "lots of comments" or "all code should speak for itself".

My philosophy is that comments should be used for two things: 1) to explain code that is not obvious at first glance, and 2) to explain the rationale or humanitarian reasons behind a bit of code that is understandable, but the reasons for its existence are unclear.

No philosophy is perfect, but I find that it strikes a good balance between maintainability of comment and code pairing and me being able to understand what a file does when I come back to it a year later.

The article is not good IMO. They have a perfect example of a function that could actually make use of further comments, or a refactoring to make this more self-documenting:

  function isPasswordValid(password) {
    const rules = [/[a-z]{1,}/, /[A-Z]{1,}/, /[0-9]{1,}/, /\W{1,}/];
    return password.length >= 8 && rules.every((rule) => rule.test(password));
  }
Uncommented regular expressions are a code smell. While these are simple, the code could be more empathetic to the reader by adding at least a basic comment:

  function isPasswordValid(password) {
    // At least one lowercase, one uppercase, one number and one symbol
    const rules = [/[a-z]{1,}/, /[A-Z]{1,}/, /[0-9]{1,}/, /\W{1,}/];
    return password.length >= 8 && rules.every((rule) => rule.test(password));
  }
Which would then identify the potentially problematic use of \W (ie: "[^a-zA-Z0-9]"). And even though I've been writing regular expressions for 20+ years, I still stumble a bit on character classes. I'm likely not the only one.

Now you can actually make this function self-documenting and a bit more maintainable with a tiny bit more work:

  // Returns either "true" or a string with the failing rule name.
  // This return value is kind of awkward.
  function isPasswordValid(password) {
    // Follow the password guidelines by WebSecuritySpec 2021
    const rules = [
      [MIN_LENGTH, /.{8,}/],
      [AT_LEAST_ONE_LOWERCASE, /[a-z]{1,}/],
      [AT_LEAST_ONE_UPPERCASE, /[A-Z]{1,}/],
      [AT_LEAST_ONE_NUMBER, /[0-9]{1,}/],
      // This will also allow spaces or other weird characters but we decided
      // that's an OK tradeoff.
      [AT_LEAST_ONE_SYMBOL, /\W{1,}/],
    ];

    for (const [ruleName, regex] of rules) {
      if (!regex.test(password)) {
        return ruleName;
      }
    }

    return true;
  }
You'd probably want to improve the return types of this function if you were actually using in production, but this function at least now has a clear mapping of "unclear code" to "english description" and notes for any bits that are possibly not clear, or are justifications for why this code might technically have some warts.

I'm not saying I'd write this code like this -- there's a lot of other ways to write it as well, with many just as good or better with different tradeoffs.

There are lots of ways to make code more readable, and it's more art than science. Types are a massive improvement and JSDoc is so understandably awkward to us.

Your goal when writing code shouldn't be to solve it in the cleverest way, but rather the clearest way. In some cases, a clever solution with a comment can be the clearest. In other cases, it's better to be verbose so that you or someone else can revisit the code in a year and make changes to it. Having the correct number of comments so that they add clarity to code without having too many that they become easily outdated or are redundant is part of this as well.

Re: Self-Documenting Code

#5
There is no such thing as universally self-documenting code, because self-documentation relies on an assumption of an audience — what that audience knows, what patterns are comfortable for them — that does not exist in general.

Self-documenting code can work in a single team, particularly a small team with strong norms and shared knowledge. Over time as that team drifts, the shared knowledge will weaken, and the "self-documenting" code will no longer be self-documenting to the new team members.

Re: Self-Documenting Code

#7

If one of my developers used "||" that way I would definitely throw some side eye

I was thinking exactly the same. You can write if (cond) { cons } on one line and get more readable code admittedly a few chars longer.

Code patterns are social! What is strange to one is normal to another.

The kind of pattern used here with the `||` might seem weird to some JavaScript developers, but it's pretty normal in shell scripts, and it's pretty normal in Ruby with `unless`!

Re: Self-Documenting Code

#8

If one of my developers used "||" that way I would definitely throw some side eye

I was thinking exactly the same. You can write if (cond) { cons } on one line and get more readable code admittedly a few chars longer.

Don't even need the curly braces. I do

    if (cond) doSomething();
all the time.

Re: Self-Documenting Code

#9

Earlier quoted context omitted.

I was thinking exactly the same. You can write if (cond) { cons } on one line and get more readable code admittedly a few chars longer.

Code patterns are social! What is strange to one is normal to another. The kind of pattern used here with the `||` might seem weird to some JavaScript developers, but it's pretty normal in shell scripts, and it's pretty normal in Ruby with `unless`!

`||` is conventional in bash, but definitely not in JS. `x || value` is closer to conventional JS, but using it for control flow is certainly not common and a stylistic choice, for sure.

Re: Self-Documenting Code

#10

If one of my developers used "||" that way I would definitely throw some side eye

I must be their target audience because as soon as they used the example with || it all started making sense.

This would have been fine too but it would trigger some people not to use {}

    if (!validateUserInput(user)) throwError(err.userValidationFailed);
My preferred style might be closer to this.

    if (!userInputIsValid(user)) throwError(err.userValidationFailed);
Post reply on HN