Code review — The good, the bad and the better
To code review or not code review?
This seems to be a topic that often circulates the internet. Often I see the topic being discussed with little context and hardly any disadvantages from proponents and hardly any advantages from the opponents.
Tip: you can improve discussions by agreeing to also research the disadvantages of your propositions. I’m sceptical when a presentation only mentions the pros but not the cons. One of the ways you can mention the cons without undermining your proposal are:
- If it is a myth, bust it in a humble, fair, concise and accurate manner
- Offer a mitigation
- Admit that it is not easy to solve and is a real contra point
- Explain how it might even be an advantage or how it might point to an entire different problem
Any way, let me start off by stating what I generally mean by a code review. A code review is a quality assurance practice that serves as a quality gate to:
- Maintain a specified and agreed upon quality standard in respect to the code base
- Create or maintain a sufficient shared (technical and functional) knowledge, understanding and responsibility among team members
- Support a dialogue among team members and beneficiaries to confirm, share and exchange ideas
- Be subconsciously more critical of your own work
Let’s first address some of the contra points of such practice:
- They (can) take (a lot of) time.
Time to review but also more time is taken to scan your code multiple times to receive a positive review or perfect score
- They can cause arguments and division
- They can cause stress for the writer of the code
In my opinion the above mentioned points have mainly to do with maturity. Not technically but in terms of soft skills.
But let’s first address the point that clearly doesn’t involve any soft skills at all. Which is the point that it costs extra time to review code.
From my experience, but I must admit an unquantifiable one, it saves time later down the road when you practice code reviews. There is a saying that speaks of this principle that I often see in my own life.
Hard work, is work not done on time.
Some things become a lot harder or more difficult to do when work is procrastinated or not done at all.
Now let’s focus on the more complex contra points mentioned earlier. I think these points have to do with the following points:
- Lack of trust and feeling unsafe among team members
- Suboptimal way of working
- Incorrect or incomplete understanding of the purpose of reviewing someone’s code
- Lack of experience and coaching to provide useful feedback
In this post I want to mainly focus on the suboptimal way of working. As this has a tremendous effect on everything else as well.
Here are some things you could incorporate in your way of working:
- Have clear and effective coding standards (this is a whole separate topic)
You could even talk about them when you are interviewing potential candidates. Don’t ask them to take it or leave it. But ask their opinion about some of the standards. Sometimes a fresh pair of eyes can teach you and the team something. It’s not a holy document so update it as the team sees fit.
- Have a checklist to provide clear rules of engagement.
- Use static analysis like SonarQube or linting plug-ins. But keep your rules up to date and provide a standard way to override some of the rules at particular places in the code.
- Be liberal about points that are really not important for readability, usability or quality. Keep taste and feelings out of it.
- Be forgiving and fix little things yourself and write a short note stating that you have done so and why. Be careful not to rewrite big portions of code.
- Offer an alternative when it needs refactoring. Either in real working code or pseudo code. But don’t just say, this is unclear, rubbish, etc.
- Start a dialogue when it concerns a functional discrepancy. Walk up to the person and talk about your findings. A great way to do this, is by asking open and honest questions. So don’t be sarcastic or rhetoric. Tip: involve the beneficiaries in this conversation to avoid theorizing and hypothesizing.
- Write a test or perform a test when you think the code may contain a bug or pose a risk. This way, when the test confirms your suspicion you can demonstrate it clearly. When it doesn’t, then you might see why.
These points will put the burden by the reviewer instead of easily pointing out all kinds of problems and putting the writer to work. Because they can only refer to a standard, fix little things themselves, think first of an alternative, connect on a personal level or write a test.
Now one of the biggest drawbacks is that it potentially costs even more time. This is true in one sense. But I believe when you adopt a similar way of working it will save you a lot of time and hardship later down the road.
Let me know what you think and whether I’ve been fair in my characterizations of the contra points for doing code reviews. Or whether I’ve missed some contra points. Or let me know when you need some advise or ideas of how to make code reviews a success in your team or company.