We Need to Change Our Attitude About Code Maintenance

Six basic rules for code reviews and comments in code

by Joe Honton Jul 23, 2019

In this episode the tech team at Tangled Web Services confronts their different opinions about comments in code.

Antoní had just finished reading yet another disparaging article about code comments, and was in no mood to let it slide this time. He talked it over with Clarissa and decided it was time to confront the issue head on. If he couldn't convince his peers out there, at least he could try to influence his own teammates.

Clarrisa opened the all-hands meeting with a few remarks. "I know you've all been working diligently to create clean code for our clients, and I think it shows, so thank you.

"But I've noticed that when bugs are found there's some reluctance to help out. The feeling seems to be 'If it wasn't my code that broke, then I shouldn't have to be the one to fix it.'

"We need to change our attitude about code maintenance. I want to have an open discussion about how we can do things better around here. And to get us started, Antoní has a few words he'd like to say."

Still seated, but looking up at Clarissa, Antoní began, "I think we are going about code reviews all wrong. There's entirely too much rubber-stamping going on. When you review someone's code you have to go beyond the rules we've adopted for our coding standards. It's not acceptable to be simply calling out stylistic stuff — poor indentation, misplaced curly braces, camel-cased names, and those types of things — they're not what we should to be focusing on.

"We need to focus on comprehension. For example, do you really understand what you're reviewing? When you're teammate is no longer around, will you be able to maintain the code?"

Devin and Ken sat firmly in their seats. As senior devs, they'd been through this before.

Ivana looked aside. Ernesto shifted his weight. Being the newcomers, they were feeling the heat.

Antoní came to the point, "And that saying that's going around, 'Not my circus, not my monkeys' has no place in our team. Besides being rude, it doesn't fix the bugs. And more importantly, it doesn't help any of us — intern, junior, senior, any of us — it just doesn't help us get better at our craft."

Ivana ventured into the awkwardness that followed, "I seem to be getting assigned lots of Jira tickets out of the blue. Ones that I'm ill equipped to handle. Wouldn't it make sense to assign them to Devin or Ken, since they know the code better than me?"

"Well, this is what I'm trying to say," Antoní replied, "If you feel like you want to hand it back to them, then something's not right with the code. Or more precisely, something not clear in its documentation."

Ivana picked up on that, "Exactly, when I read your code I have a better sense of what's going on. But when I read code without good comments (or code without any comments), I have to puzzle my way through it, and everything slows down."

Devin could see where this was going, and launched into defensive mode, "It all sounds good, but what happens when you read the comments and they are wrong? How often have you seen comments that were written so long ago that they no longer apply? Or worse, comments that seem to apply but are inaccurate, and send you off in the wrong direction."

He paused for a second, before hitting his stride, "How many boilerplate comments are crufting up our code base? How many TODOs are never going to get done? How many refactored and commented out sections are littering things up? I don't call that clean code!"

Before anyone could rebut the argument, Ken plowed in. "I'm firmly in the camp that believes that clean code needs no comments — it should be self documenting. Use good variable names and use functions that say what they do. If something's broken, then read the code and fix it."

Devin and Ken had been around, so both of their convictions were strong. But Ivana was wavering, and looking at Antoní said, "How about just assigning me the Jira tickets related to your code? I'm happy to work on those. Let the other guys take the Jira tickets for their code." (Somehow Ivana found Antoní's code easier to understand than either Devin's or Ken's.)

"No," Antoní was flummoxed that he wasn't making himself clear. He tacked into the wind, "That's exactly what we don't want to do.

"Here's the deal, I want everybody to be able to understand our collective code base. You don't need to be an expert, but you at least need to be able to read it and comprehend what it's doing. There shouldn't be my code and your code. It's our code, period."

Then he laid down the law, "If we aren't commenting our code well enough for others to be able to read it and comprehend it, then we'll have to institute pair programming."

Silence. Nobody wanted to go there.

Ken, who was the clear leader of the no-comments-in-code gang, gave it one more feeble attempt. "So are you saying that we need to document every function, and every variable going in and out of every function? And when I write an accessor like getUserName I have to spell out what the function does and what it returns? And when I create a module that holds enumerated values, I have to put a comment on each one, even if the names are SUCCESS, SUCCESS_WITH_DATA, and FAILED?"

Antoní chose to ignore his flippant remark, and answered him this way instead.

"Here are some of my guidelines:"

1. Start with the attitude that you're writing comments for yourself, not for others, then you won't end up being pedantic or long-winded.
2. If there's enough context to understand what's going on without a comment, then don't feel obliged to write one.
3. If a comment doesn't impart complementary knowledge, it isn't doing its job. Comments shouldn't simply repeat what the code says.
4. Before submitting a pull request, take a second look at your comments to make sure that they accurately depict what you've just created.
5. When you review someone else's code, review the comments too. Send it back if the comments aren't clear.
6. When you're fixing a bug, and you see that a comment is wrong or confusing, fix it. If it really doesn't need to be there, be bold and delete it.

"For me, the code and the comments aren't two separate things. They're symbiotic."

"Symbiotic?" Devin chided him, "The comments benefiting the code I get, but the code benefiting the comments? Really?"

"OK," Antoní knew he had stretched it a bit too far, "Complementary. How about we go with that."

That helped cool the temperature in the room a bit.

The only one who hadn't chimed in was Ernesto. Being the newest one on the team, he was sitting quietly throughout the whole exchange, hoping that no one would call him out. Secretly he felt that he needed to do better, but at the same time he had some of the same misgivings about comments as Devin and Ken. He was glad to hear Antoní describe things in shades of gray, rather than black and white.

---

Clarissa and Antoní lingered on for a minute while the others went back to the trenches. "So, how long do you think that pep talk will last?" Clarissa asked.

"Probably about two weeks, then the guys will be back to their old ways again. Still, I'm hopeful for the younger ones. There's a chance I'm going to get through to them."

---

No minifig characters were harmed in the production of this Tangled Web Services episode.