My random thoughts about software engineering

An aspiring software craftsman journey, By Mahmoud Ben Hassine

A simple rule (inspired by chess masters) to improve code reviews

Chess masters do game analysis and review after every single game they play. They are eager to learn from every single mistake they make. This is what makes them masters! When two chess masters do a game review, you may hear them saying:

  • “Black in this position has a slight advantage in my opinion” or
  • “Starting from this position, white has serious problems” or something like
  • “Black could not survive bishop attack here, that’s why he resigned”

The point is that game review is strictly about the game, the position, the moves that were played, but never about the player himself.

In “Starting from this position, white has serious problems”, “white” could be the chess master himself! He didn’t say “… I have serious problems”. Likewise, in “Black in this position has a slight advantage”, “Black” may refer to the opponent, He didn’t say “… You have a slight advantage”.

Here are some examples from the press conference of the world chess championship 2016 between Magnus Carlsen and Sergey Karjakin:

  • "Probably Black should play Qd7 immediately", says Sergey about Magnus, playing black (2:35)
  • "Something like this would be a huge achievement for white", Says Magnus about Sergey, playing white (7:15)

You've got the point, the review is about the game, not about the player.

This is exactly how code reviews should be conducted between developers. The review should be about the code, never about the coder. Unfortunately, some code review sessions turn into personal attacks and may create trouble in the team and sadly impact the whole project.

I have worked with nice people who know how to apply code review’s best practices to be constructive and helpful. But I have also heard:

  • “Your code sucks!”
  • “Please stop writing ugly code”
  • “What the fuck is this shit?”

That was not necessarily about my code, but it’s true! and happens quite often sadly.. even between top notch developers (here are some examples from Linus Torvalds doing a code review of a pull request to the linux kernel).

Anyway, like chess masters, we should hear two code masters saying:

  • “In this situation, this code could be better extracted in a separate class” or
  • “In my opinion, this code violates the Single Responsibility Principle” or something like
  • “I think we can refactor this snippet to apply the strategy pattern, it will be more flexible. Do you agree?”

What a wonderful discussion! What a pleasant code review! The reviewer and the reviewee are both speaking about the code, not persons.

To conclude, two points to note:

  • When you do a code review, please do not refer to the coder in person
  • When your code is being reviewed, please don’t take the review personally

This is actually a very simple rule, and if we apply it, our code review sessions could be really better.

Let me leave you with some great references on code reviews, all referring the simple rule discussed in this post: