Contents

Doing code reviews means interacting with other humans, which we need to take into account. We also often have the luxury of being able to choose the reviewer, and the obvious candidate is not always the best choice.

It’s personal!

We developers can be a strange breed. Despite the cold logic of the computer we have to deal with, programming can have to do a lot with emotions. We’re often proud of what we write, especially when we solve a large or tricky problem in an elegant and consistent way. Programmers also pour considerable creative effort into their code, up to the point where some even compare it to art. Code reviews can feel like your abilities are put to scrutiny, no matter how much we affirm that it’s only about assuring code quality.

The official credo often is that the code of a project is the team’s property and responsibility and does not belong to a single developer. However, code reviews are the occasions where we make it so, where we give our own creations over to the rest of the team. Unless we do pair programming, code reviews are the first occasions where someone else gets to look at our code. If it then has to be rejected, it can leave a bad feeling, so it is important how we deliver the bad news. Especially with less experienced developers, we have to be careful to not be too picky.

Be nice

The most important thing is to check ourselves that we review the code, not the person. When we go through the changes, we have to focus exclusively on the code and not keep in mind who wrote it. That means not scrutinizing a junior developer’s code more than that of a senior dev, and not giving the resident guru more slack than anyone else.

Often we come across syntactic details we would have done slightly differently. Keep in mind that the semantics are far more important, and if the code is not far less readable and understandable than alternatives, it’s just as good. You may still ask about it because maybe there’s some detail to learn you didn’t know yet, but it should be far down in the list of priorities.

A good way to not come across too harsh is asking questions instead of asserting bad code. Consider, for example, “You did not check for division by zero here” versus “I did not see any check for division by zero, is it not needed?”. The latter is far less confronting and gives the reviewee time to think about it and find the problem themselves. Besides that, it is also less awkward if we are the one who missed the check for zero or the reason that it actually is not needed.

LGTM

Some people take being nice and quick a bit too far and just give a stamp of approval to whatever they have to review. To shorten the review comment, a simple “LGTM” is all they’ll ever type. Rumor says that this practice is not only common among the nice, but also among the lazy and refuseniks.

“LGTM” may be the right thing to do for certain changes, think e.g. of a fixed typo in an environment where every single change has to be reviewed. However, on larger changes, it is suspicious at best. Simply waving through every change is dangerous, since you basically skip the quality gate that code reviews are. Approving flawed code makes the reviewer as responsible for errors as the original author.

Even if there is no apparent error and it’s OK to approve the change, a few more words don’t hurt. There’s almost always something you could say about the code. Is there something the reviewee could learn? If there’s no big issue in the code, there usually is something small you can comment on, while still approving the change. And if they actually happen to have produced a batch of perfect code, congratulate them for it – remember, we like to be proud of our good work.

Who should do the code reviews?

It is completely up to you and your team whether code reviews are done by a single person, or by two or more, together or independently. It’s more important to have the right people do the reviews, and ideally, to have everyone on the team do a code review one in a while. Only that way everyone has the opportunity to learn from everybody else and benefit from their experience. Therefore, do not always fall back to the resident guru to do your code reviews.

Even if your process requires you to have only a single reviewer, that should not deter you from asking a second person to do code reviews for you. For example, I recently have written C# code to generate some C++ for embedded devices. I usually asked two people for code reviews: One C# expert, as I am not very experienced in using it, and one colleague who knows a lot more about embedded programming than I and the C# expert do.

Senior or junior

Both seniors and juniors may be good candidates for a review. A more senior developer may be more knowledgeable in the business domain, or in the programming language that you use. Juniors benefit from reading code others have written, but they also often can provide different insights they have recently learned. The also often have a less biased way to think about the problems the software solves.

Non-developers

Code reviews do not always have to be done by other developers. It can be a good idea to have a test engineer look at your code, too. I even have asked business experts for code reviews when I was writing business logic in a domain-specific language. It was C++ code they had to look at, and they did not know anything about C++, but the DSL was clear enough for them to see whether the business logic was right, once after they got past the syntax.

Review yourself

Even if you are not asked to do a specific code review, that is no reason to not do it if you are interested in the solution to a specific problem. Curiosity is not a crime and looking at how a colleague has solved a tricky problem is a great learning opportunity. You may even have something to contribute. Make sure, however, that it’s ok for the author and the team.

Group reviews

For very interesting problems or code that contains important design and architectural decisions, it can be a good idea to do code reviews with a whole group of people. Typically, the author should present the changes and the group will focus only on specific aspects of the code. Therefore, a group review is not a replacement for the normal solo code reviews, but an addition.

Conclusion

I have now rambled on about code reviews for five blog posts – but there still is much more to explore and be said about it. Have you made your own experiences with code reviews, good or bad? Please share in the comments!

All emojis by emojione.com

Write clean and maintainable C++

Code Reviews – The Human Aspect (Part 5)