corporate culturequality assurance

Code Reviews Done Right

Recently, I’ve been working with a project team embracing code reviews. The experience wasn’t pleasant. Most of the time, it was downright annoying.

The general idea of code reviews is that one of your team members dedicates several hours of their precious time to look at your code and help you to improve it. That’s a generous offer. Plus, hardly anybody rejects an opportunity to learn and improve their coding style. I, for one, definitely don’t. So how come the code reviews were such a pain in the ass?

Be constructive

Most of the time the code reviews were about coding style. Instead of really analyzing the algorithm, they criticized rather formal aspects of my code. I often found it hard to counter the argument. But my general feeling was that the suggested improvements made the source code worse instead of improving it. For instance, two popular strategies of the team were to use lambda expression wherever possible and to split the code into countless methods. More often than not, I recommend the same strategies to new programmers, but there are reasons not to exaggerate it. One of those reasons is debugging. As to my perception, it’s often difficult to debug a complex expression using lambdas and functional programming. Short functional expressions are no problem, but if you keep chaining lambda expression, there’s a tipping point. After that, things get ugly.

Similarly, splitting the code into methods and functions improves readability and helps you to debug the code. But only to a certain point. Stepping into a dozen nested methods during a debugging session exceeds the capacity of the average programmer’s brain.

Take the opinion of the programmer you’re reviewing into account

So most of the code reviews yielded results I considered weird, to put it mildly. However, that wasn’t the annoying part of the story. The annoying part was that the reviewers insisted on implementing the suggestions. If I didn’t do that myself, they did it for me.

The better approach is to convince the other programmer. Maybe they’ve got a good reason for selecting a particular approach. Granted: most of the time they are just sloppy or thoughtless. But it’s unlikely they are always sloppy and thoughtless. Sometimes it’s the reviewer who’s learning to write better code.

Don’t forget the human factor

In other words: code reviewers have to deal a lot with emotions. You’re criticising the work of your colleague. They’ve spent a couple of hours writing an algorithm, and you’re telling them their code is garbage. Do you really expect them to agree happily? They won’t, even if you’re right.

Wrapping it up and further reading

The secret of successful code reviews is to do them in a constructive way, showing respect to the effort of your colleague. For some reason, that’s often neglected. Michael Lynch has written a great post on the human factor of code reviews. You’ll both laugh and learn a lot reading this article. Highly recommended!