DEV Community

Discussion on: "Do not comment on your code, it should be self-documented". Well... I don't agree.

 
klausdonnert profile image
Klaus Donnert

You made that up. Nobody is that bad. I did get a chuckle out of it.

Thread Thread
 
chasm profile image
Charles F. Munat

It was a joke. But it's not as far off as you might think. :-)

Thread Thread
 
klausdonnert profile image
Klaus Donnert

I consider comments as documentation. If a programmer does not document their code... I don't have energy to finish that sentence.

Thread Thread
 
chasm profile image
Charles F. Munat

Comments are a shit way to document code. The best way is in the code itself. So if your code is self-documenting, then you have documented your code. The only excuse for comments is that you had to do something in the code that you can't figure out how to make clear without a comment.

Maybe it's a workaround. Maybe you're just not that good. That's why we have teams -- so they can show you how to write better code.

In short, comments are generally where mediocre coders document their failure to write understandable code. It's either that, or the comments are redundant and probably just get out of sync.

So one could say that the more you comment your code, the more you're willing to admit that you don't write very good code. And if that's the case, then I guess comments are better than nothing. But why not learn to be a better coder or take up a different profession?

But hey, black and white condemnations like yours are all the rage these days. Maybe see a doctor, though, about your anemia?

It is always surprising how many devs confuse their personal preferences and pet peeves as scientific arguments and absolute judgements. And then boast about it.

Thread Thread
 
klausdonnert profile image
Klaus Donnert

Apologies. Sir, I do not condemn self documenting code. Most of the code I write is self documenting. We agree that bad code is bad code. You make a solid argument that comments don't improve bad code.
I generally use comments in a couple of ways. One is when I am writing a function or method, I write the steps out in plain English as comments before I implement them. Then I remove redundant comments. Some comments I leave because some things are nuanced and not obvious.
The second way is explaining, usually to my future self, why I did something a particular way, not what the code is doing, because well, I'm not as smart as I think I am. Even then, most of those comments are from my future self to my future future self to save time the next time I have to modify it.
Third, is explaining somebody else's code for instance, where they used a variable or parameter named "id" where id could be the column of one of 6 tables, and refactoring is not an option because it's spaghetti code in a huge legacy codebase.

I'm interested in your work flow. You're a teacher. I'm a student. How do you do it?

Thread Thread
 
chasm profile image
Charles F. Munat

Sorry, I misread your last comment as saying that all code had to be commented. I rarely comment my code.

Instead I:

  • Keep functions short -- 10 to 20 lines max, if possible
  • Limit functions to doing one thing only (though that thing might involve conditional outputs)
  • Name the function by what it does; name length is much less important than clarity
  • Keep my functions pure and referentially transparent
    • Hence, a function with zero parameters is a constant (e.g., function returnFalse() { return false }
  • Use options parameters
    • If the number of parameters will exceed 3
    • If I want to avoid positional parameters
  • Return tuples or objects if I need to return more than one thing
  • Always use a return statement (unless an arrow function)
  • Prefer named functions; arrow functions are for:
    • Passing an anonymous function in the parentheses of a function call
    • Controlling the this, typically in callback functions (anywhere you might use const self = this)
  • Use TypeScript (waiting for a better type system) for static typing
    • No any
    • type not interface -- interfaces are mutable
    • Avoid unknown in production code

Also:

  • I never use classes: I use modules (ESM)
  • One function (or component) per file
  • Name of the function or component (camelCase or PascalCase, respectively) is the name of the folder
  • Filename is either index.ts(x) or mod.ts(x)
  • All files associated with that function/component/module go in that folder, e.g.:
StringField/
  index.module.css
  index.stories.tsx
  index.tsx
  index.test.tsx
  README.md   -> the exception
utilities/
  not/
    index.test.ts
    index.ts
Enter fullscreen mode Exit fullscreen mode
  • I typically use module aliases so I can import from the top down: import not from "~utilities/not"
  • I don't use the bang (!) because it is too easy to miss; instead, I always write a generic not function and use that
  • I use double quotations because they are easier to see (I'm writing TypeScript)
  • I don't use semicolons because they just add noise -- in a decade of leaving them out, ASI has never once let me down, and now with the TSC compiler and using deno_lint to lint...
  • I use tabs for indentation
    • They use fewer characters
    • Everyone can set their own damn tab width
    • I'm using Dprint anyway
  • I prefer and use almost exclusively the default export option (exceptions: types, constants)
  • I always use import type for type imports, it's cleaner
  • I like one import per line where possible and I use an import sorter
  • I use VSCode currently, so I can easily collapse the imports
  • I put the default export function at the top of the file just below the imports so it is immediately visible
  • Using named functions means I can put helper functions below the default function as they're hoisted
    • But I generally put helper functions in their own folders
  • If a type declaration might be reused, I add it to a types file at the top of the module with like types, otherwise I keep it below the function/component but export it as a named export just in case
  • I deeply nest folders (remember: all names are on folders, not files)
    • At the top of the app (I do front end apps), I generally have:
      • An apps folder
      • A utilities folder (for generic utilities such as not, pipe, identity)
      • A services folder for any bespoke services (i.e., not in an npm or deno module) such as authentication
      • A modules folder for any reusable modules (e.g., generic components, icons)
  • IMPORTANT: If I have a function used only in one folder, then its folder goes in that folder (as a child folder), but if I then begin using that function in some other branch of the folder hierarchy, then I move its folder to the node where those two branches meet
    • In short, imports can go up the folder hierarchy, but they don't go back down.
    • And, often, I just do all imports not in a descendent folder from the top down, e.g., import doIt from "~apps/TicTacToe/utilities/doIt" rather than import doIt from "../../utilities/doIt"

In the apps folder, I have "micro-apps". These are standalone apps. Everything bespoke to that app is in that folder. Everything outside of that folder is generic and reusable.

So, for example, I may have a services/useGraphQL folder that provides a hook for using GraphQL, but it takes a config and returns query and mutation functions. So the actual URL, query, variables, etc. are provided where they are used. None of this is hard coded in the useGraphQL hook. (And I don't bother with Apollo -- a simple POST request returning JSON works fine.)

Inside the apps folder, I might have a micro-app called, I dunno, TicTacToe. The hierarchy of that folder and its subcomponents would follow the hierarchy of the components, for example:

apps/
  TicTacToe/
    Board/
      Square/
        index.tsx
      index.tsx
    index.tsx
Enter fullscreen mode Exit fullscreen mode

The benefit of this is that:

  • I can close up folders and see clearly the structure of the app
  • The structure of the folders matches that of the app, reducing cognitive load
  • PascalCase or camelCase tells me immediately what is a component and what is a function, repectively
  • The .tsx ending tells me it is using JSX (I am forced to use React usually, though I prefer SolidJS or even plain vanilla TS -- deno gives me JSX for free)
  • Most importantly, if I want to remove TicTacToe, I just delete this folder (or move it elsewhere) and remove the typically one reference to it (<TicTacToe />) elsewhere in the app
  • The TicTacToe micro-app can import from any of the other apps (using aliases), but other apps do not import from the micro-app (except to use it as an app, e.g., in a route)
  • I keep all the business logic and bespoke bits in the micro-app

I do not care how short a file is. Why would I? For some reason, many devs fear files. I don't get it. Why would I make a 1000-line file full of named exports when I could make a well organized folder tree with maybe 20 files, each of which contains a single function? If I need to view multiple functions at once, I can open them in side-by-side tabs.

Here is an example of the not function I mentioned, from production code:

export default function not<T>(value: T): boolean {
  return !value
}
Enter fullscreen mode Exit fullscreen mode

That is the entire file! Three lines. Here is a somewhat longer one:

export default function concatenateCssClasses(classNames: {
  [className: string]: boolean | undefined
}): string {
  return Object.entries(classNames)
    .reduce(
      (classList: Array<string>, [className, include]) => [...classList, ...(include ? [className] : [])],
      [],
    )
    .join(" ")
}
Enter fullscreen mode Exit fullscreen mode

That replaces an entire dependency (on "classnames")! You can see pretty easily, I think, that it takes an object with CSS class names as the keys and booleans as the values, and then includes only those that are true, concatenating them into a space-separated string. And if that's not enough to be clear, then in the very same folder is the test:

import { expect, test } from "vitest"
import concatenateCssClasses from "./"

const classNames = {
  red: true,
  green: false,
  blue: true,
  "burnt-sienna": true,
}

test("[concatenateCssClasses] creates a space-separated string of class names for className keys with truthy values", () => {
  expect(concatenateCssClasses(classNames)).toBe("red blue burnt-sienna")
})
Enter fullscreen mode Exit fullscreen mode

Other than a few polyfills for Intl and Temporal, fetch, and uuid, my app uses only React, XState, and, sadly, Auth0.js (not my choice).

"dependencies": {
  "@formatjs/intl-getcanonicallocales": "^1.9.2",
  "@formatjs/intl-listformat": "^6.5.3",
  "@formatjs/intl-locale": "^2.4.47",
  "@js-temporal/polyfill": "^0.4.1",
  "@xstate/react": "^3.0.0",
  "auth0-js": "^9.19.0",
  "cross-fetch": "^3.1.5",
  "react": "^18.0.0",
  "react-dom": "^18.0.0",
  "uuid": "^8.3.2",
  "xstate": "^4.31.0"
}
Enter fullscreen mode Exit fullscreen mode

I write my own code, including utility functions, and reuse it from app to app, improving it as I get better (and the language improves). So yes, I write my own pipe function, and often map, filter, find, reduce and more (wrapping the JS methods where appropriate).

That means that I know practically my whole code base (an argument for vanilla TS). It means that to the greatest extent possible, no one else has code in my code base, which means better security, better reliability, etc.

It means that when things break, I know where they broke and why they broke, and I can fix them rather than waiting for the owners to get around to fixing them and releasing a patch.

It means that I am highly motivated to keep my code base simple, not to bulk it up with unnecessary and bloated dependencies.

It means that most of my files can be seen in full without scrolling.

And if my code needs further documentation, I try to put it in those README.md files right in the folder (README means GitHub will automatically display them).

That's just a start, but I hope it answers your question at least a little.

Lots of devs violently disagree with one or another of the above, and I have done all of these things differently over the years, but this is the methodology that has stood the test of time. I'm sure it can be improved still further, and significant changes in language, framework, or library might make adaptations necessary, but I can say that of the many people I've taught this too, none have gone back to their old ways. It's simple, and it works.

YMMV.

Thread Thread
 
klausdonnert profile image
Klaus Donnert

Good answer. That's more than I expected. It will take me some time to digest it all.
I struggle keeping my functions short. 50 - 100 lines is not unusual for me. Three line functions always make me second guess myself, "Should I inline this"?
Earlier, I was thinking to myself "I bet he uses readme files".
Your use of utilities is fascinating. I have some functions that I seem to redefine over and over in different projects. This a good way to organize then and not redefine them.
I've used the folder/index.* naming convention in one project. It confuses me a bit and make my tabs too wide when I get several files open at once.
The temporal polyfill is interesting.
I like writing vanilla js for much the same reasons you use vanilla TS.

Good Night

Thread Thread
 
flutterclutter profile image
flutter-clutter

@chasm
Just wanted to say that I agree with almost every statement. I have made very similar experiences during my time as a software developer and have come to very similar conclusions. Most of the things are in line with Uncle Bob's Clean Code. Thank you for the detailed elaboration!