Comments in your code are the simplest and often most useful means of documenting your decisions and communicating intent for those that need to understand it in the future.

However, one must be careful to ensure that the comments one leaves behind are not noisy or misleading. My general philosophy when it comes to comments is as follows:

Never tell me what, sometimes tell me why, always tell me why not.

As with all things in software, this principle is not hard and fast, but a general heuristic.

The easiest way to explain this is by example. Imagine we are writing a small helper function for filtering inactive users out of a list of users we are pulling from our database.

Never tell me what

Do not explain concepts or approaches that a seasoned programmer looking at the code can plainly understand. This is something I often see novice programmers do - they project their lack of comprehension of the basics onto the reader in the form of over-documented code.

const getActiveUsers = (users: User[]) => {
    // Iterate through each of the users and check whether their `isActive` property is true.
    return users.filter(({ isActive }) => !!isActive)
}

This comment is useless for anyone with a moderate understanding of Typescript syntax and idioms. It is the bread and butter of the language.

Sometimes tell me why

Usually, when you are doing something that violates the Principle of Least Surprise, you should consider explaining to me why you’ve made that decision.

const getActiveUsers = (users: User[]) => {
    // We need to filter out inactive users here because we don't have a DB index on the `isActive` field. Filtering here keeps latency down at the expense of less full, or potentially empty pages
    return users.filter(({ isActive }) => !!isActive)
}

This is significantly better. As a reviewer, I understand why we’re doing something nasty that should really be handled by the database.

If I come back to this in 6 months and the index has been provisioned, I now understand why we needed this and why it’s no longer necessary. I am empowered to remove this nasty code because I understand why it existed in the first place.

Always tell me why not

If there is an obvious solution to a problem which you have deliberately avoided, you should definitely tell me why you’ve done that. Most of the time, when I’m reviewing code with “tell me why” comments, my immediate next question is going to be “why not X?”, if X is an obvious solution to the problem.

By adding it to your comment, you’re helping me as a reviewer, and the next person who has to read this understand why you avoided the obvious fix.

const getActiveUsers = (users: User[]) => {
    // We need to filter out inactive users here because we don't have a DB index on the `isActive` field. Filtering here keeps latency down at the expense of less full, or potentially empty pages
    // Ideally we'd just provision the index and remove the need for this. We're adding this as a stop-gap measure as a backend deploy is quicker than a migration to add an index. Adding the index is tracked in XXX-123
    return users.filter(({ isActive }) => !!isActive)
}

This is extremely useful as I now understand why you’ve avoided the obvious fix. If I come across this in the future I can track whether the obvious fix was ever actioned, and if not, potentially action it myself.

I have expressed the sentiments in this post a few times throughout my career, and pretty much always in the terms described above. This section is a new entry to the philosophy owing to a particular conceit of LLMs: using comments as a way of communicating with their handler.

These are probably the worst comments possible as they offer no signal beyond the session in which they were created, are stale by the time the code in question is reviewed, and are putrid by the time another reader comes across them. They look like this:

const getActiveUsers = (users: User[]) => {
    // FIX: The database layer returned inactive users, filtering them here stops inactive users appearing on the admin dashboard.
    return users.filter(({ isActive }) => !!isActive)
}

This gives me a big dose of what, a small side serving of why, and absolutely no “why not”.