Most developers starting out assume comments are like sprinkles. They're something everyone likes and the more you use it, the prettier your donut. One of my favorite authors Robert C. Martin, author of "Clean Code", has this to say about comments.
"Usually they are crutches or excuses for poor code or justifications for insufficient decisions, amounting to little more than the programer talking to himself."
Given this context, it's not surprising that junior devs write so many comments, since their code tends to be pretty rough around the edges. Even veteran developers who can truly write beautiful code sometimes think they need to reach for comments.
I'd like to make a case against this impulse.
How comments are used
To explain what is already obvious
There are plenty of times I've seen developers write perfectly simple code only to litter it with comments that explain what was already obvious.
export async function checkUserPermission(permission: Permission): boolean {
// if user is not authenticated, throw error
if(!auth.isAuthenticated){
throw new UserNotAuthenticatedError()
}
// get current user role
const currentUserRole = await userService.getUserRole(auth.currentUser)
// check if current user role includes permission
return currentUserRole.hasPermission(permission)
}
To hold on to dead code
Many devs can't help but comment out code they're afraid to delete.
// const hasPermission = await checkUserPermission(permissions.posts.edit)
// if(!hasPermission){
// router.reject('insufficient access')
// }
To explain strange business logic
Even the most eloquent engineer might struggle to explain particularly complex or misleading business logic.
function calculateLoyaltyMemberDiscount(customer:Customer) number {
if (customer.isLoyaltyMember) {
// loyalty members discount is 1% per month before merger, max 5%
return Math.min(monthsInInterval({start: new Date(2010, 1, 1), end: customer.startDate}) * 0.01, 0.05)
}
}
To document code
Especially in situations like projects that use vanilla JS, tools like JSDoc can provide some seriously useful context that happens to be provided in the form of comments.
/**
* Calculates the loyalty discount for a customer based on their membership duration.
*
* - Loyalty members receive a 1% discount for each month they were members before the merger.
* - The maximum discount is capped at 5%.
*
* @param {Customer} customer - The customer object.
* @param {boolean} customer.isLoyaltyMember - Indicates if the customer is a loyalty member.
* @param {Date} customer.startDate - The start date of the customer's loyalty membership.
* @returns {number} The calculated loyalty discount.
*/
function calculateLoyaltyMemberDiscount(customer) {
...
}
Why comments are evil
Most comments are added in an effort to improve readability, but what's evil about comments is they inevitably do the exact opposite. Even the best, most clearly written comment will eventually become evil, serving the demise of the code it was sworn to protect.
Comments can move
Just like in real estate, comments are all about location. As blocks of code get moved, lines added, lines removed, the location of a comment might get moved away from the block it was referring to.
Comments can be outdated
Codebases are constantly changing. Functions get renamed, conditions get added, APIs get deprecated, bugs are removed, bugs are added. How much do you trust other engineers, or yourself in 6 months, to make sure to refactor comments alongside the code that the comments refer to?
Comments can be misleading
Now imagine that a refactor happens and nobody catches the outdated comment in code review. That comment is now not only wrong, it's misleading. Future engineers will spend more time trying to figure out how the code ever worked than they would if there was no comment at all.
How to stop using comments
Fortunately, the problem of explaining our poor code has better solutions than adding comments. Most of the time code is capable of explaining itself, it just takes a little practice.
Give your variables, parameters, functions clear names
Avoid single letter variables, acronyms, or shortening in general. There is no good reason in modern development to not be verbose.
function exec(cb) {
let t;
return (...args) => {
clearTimeout(t);
t = setTimeout(() => cb(args), 2000);
};
}
function debounce(callback) {
let timer;
return (...args) => {
clearTimeout(timer);
timer = setTimeout(() => callback(args), 2000);
};
}
Break things out
Some developers seem to think that you should only break out a function, component, file, if you think it's in pursuit of DRY code. Or in other words, only if it's likely to be reused. That's more of a convenient side-effect of the primary goal, keeping blocks of code small and concise!
Encapsulate primitives and conditions with clear names
Whenever you see a primitive, a ternary, an if statement, or any other condition, see if breaking out a variable would make the intent clearer.
// 42 is the offset of the header
scrollTo(42)
// if animal is a dog
if('sound' in animal && animal.sound === 'bark'){
...
}
const headerOffset = 42
scrollTo(headerOffset)
const animalIsDog = 'sound' in animal && animal.sound === 'bark'
if(animalIsDog){
...
}
Use unit tests
Even with all of the tips above, some bits of business logic are too weird to explain easily in code. Complexity like this is a great candidate for a unit test. The title of your unit test can literally be the comment you were going to write, but a good test lets you assert the behavior doesn't change without mucking up the production code itself.
Example
Let's look an example of some seriously ugly code and clean it up using the suggestions above.
Before
type Usr = {
id: string,
perm: number[],
}
// validates user has API access and active session
const v = async (x?:Usr) => {
if(!x) return false
// user must have API access
if(!x.perm.includes(1)) return false
// check session is active
return await fetch(`/usr/${x.id}/valid`).then((rsp) => {
const valid = rsp.json()
return valid
}).then(valid => {
return valid.valid
})
}
After
const USER_PERMISSIONS = {
API: 1,
ADMIN: 2,
} as const
type UserPermission = typeof USER_PERMISSIONS[keyof typeof USER_PERMISSIONS]
type User = {
id: string,
permissions: UserPermission[],
}
const validateUserHasActiveApiSession = async (user?:User) => {
if(!user) {
return false
}
const userNotSignedUpForApi = !checkUserHasApiAccess(user)
if(userNotSignedUpForApi) {
return false
}
return await checkSessionStillActive(user)
}
const checkUserHasApiAccess = (user: User) => {
return user.permissions.includes(USER_PERMISSIONS.API)
}
const checkSessionStillActive = (user: User) => {
return UserSessionApi.checkUserSession(user.id).then(({valid}) => {
return valid
})
}
class UserSessionApi {
public static checkUserSession(id: string): Promise<{valid: boolean}> {
return fetch(`/usr/${id}/valid`).then((response) => {
const sessionValidityStatus = response.json()
return sessionValidityStatus
})
}
}
This rewrite might be longer, but each block is tiny and significantly easier to understand and as a result maintain. Best of all, as the code evolves there are no comments that will get moved out of place, outdated, or misleading.
Comments in code is a tool, you should use all the tools at your disposal to make the cleanest code base possible. However, it's important to understand that all tools have their downsides. So, next time you think your code needs a comment, see if you can fix it without a comment.
Top comments (0)