The joy of code review

I love code reviews. I love sharing my solutions with others and receiving their feedback. I love seeing what others have developed and finding ways to improve it. I love thinking about code quality and debating it with others. In this blog post, I hope to pass a little bit of that love on to you.

I will examine some coding best practices and see how they can be applied to code review itself. I'll borrow inspiration from Uncle Bob Martin. I agree with many of his ideas and they make for some good quotes. I have experience working in a product oriented, mid-sized development team. Ideas I'll be discussing relate to a 5 – 10 person team reviewing each others code that's mostly constrained to a handful of products they own.

It's not about catching bugs

The secondary value of software is its behavior. ... The ability of software to tolerate and facilitate such ongoing change is the primary value of software.

Uncle Bob

I really like this idea of Uncle Bob's: that fulfilling immediate requirements is secondary to facilitating the future changes. I believe similar can be said of code reviews. The secondary value of code review is to improve the quality of a given code update. Its primary value is to improve all the future code updates!

This commitment to long term improvement means that code reviews should be a learning opportunity for everyone involved. The goal is no longer just to identify bugs, but to teach each other how to produce better code.

In order to achieve this, reviewer should use their comments to educate. It's not enough just to point out deficiencies. They should propose improvements along with the reasoning for them. The reviewer can be helped in this by a shared set of team values and a common language.

The set of shared values can be as simple as everyone on a team agreeing that they should write clean code. This is where common language kicks in by insuring everyone's idea of clean code is the same. Common team language is also valuable outside code reviews and there are techniques for building it. One technique I like is a book club in which team members read the same technical book and meet every few weeks to discuss the latest set of chapters. Another technique is the architecture drawing exercise I wrote about before. All this allows reviewer to, for example, simply reference one of the SOLID principles as a motivation for their comment.

Education goes both ways! One way for code author to educate the reviewer is by following good coding practices. They can, however, do more. While comments in code itself are undesirable, I find them useful in a code review. Author of the code can leave comments in the pull request that can be educational for the reviewer.

I find this a good method for teaching design patterns. They can often be hard for junior developers to grasp. While books on the subject do contain examples, those can be hard to relate to. When a code author introduces a particular design pattern in a pull request they can leave a comment with a link to a theoretical explanation of the pattern. This way a junior reviewer can learn the pattern by seeing it applied to a code base they're familiar with, and solving a domain problem they understand.

Encourage good behavior

Leave the campground cleaner than the way you found it.

Uncle Bob

There are some good behaviors you may wish to encourage in your team. For me a good example of that is the famous boy scout rule: always leave the code just a little bit better than it was when you found it. This rule is easy to express and can be easily recognized by a human. Yet, it's somewhat hard for automatic checks such as liters to detect and enforce it.

This makes it a perfect consideration during code review. Reviewer can call for boy scouting where author may have missed it. However, they have an even greater opportunity: to praise the improvements already submitted by the author! This is a handy way to give immediate positive feedback on a desired behavior. And it's a damn effective one. If you've ever received a comment praising your pull request you'll know how good it can feel when someone else recognizes your hard work and commitment to good practices.

Review for readability

The ratio of time spent reading (code) versus writing is well over 10 to 1 ... (therefore) making it easy to read makes it easier to write.

Uncle Bob

Code review is often the first time someone besides the author reads the code. Simultaneously, this is also the time when it's easiest to change the code. It's still fresh in author's mind, it's not even in the master branch yet, no one is using it. This combination can be a powerful one.

Junior reviewers can have an advantage here. Intermediate level developers can have a tendency to over-complicate their code. This is a simple byproduct of their expanding skill set. Having a junior reviewer on a such pull request can be helpful as they can call it out. Sometimes a complex chain of callbacks can be rewritten as a simple for loop that will be easier to understand for everyone.

Code review is also a good opportunity to get buy in from another member of the team. If maintenance and future development is shared between all team members it's important that everyone accept and understand the author's code as if it was their own. Code review is the first time this happens. We should encourage reviewers to express their confusion with the code. If they don't understand the code today, when they are seeing it in isolated pull request and they have time to examine it, then they won't be able to troubleshoot it tomorrow, when it's integrated with the code base and they are under pressure to fix an issue.

Don't automate away the humanity

Programming is a social activity.

Uncle Bob

There are aspects of code review that can and should be automated. Things like code formatting, compiling and linting. By all means add git hooks to ensure uniform code style. Hook up CI pipeline to build the project and run tests on each commit. In my team we agree to start the review only after Jenkins job has passed.

All these tools help automate repetitive steps and make reviewer's life easier. However, I'd caution against relying on them exclusively. I still need a fellow developer to check the readability of my code. Static code analysis can gamify development, but it can't replace the recognition and encouragement of my peers. There's no substitute for discussion of coding practices and learning how to apply them that might happen during a review.

Quality code review requires time, commitment and skill. Reviewer needs to approach it with dedication and seriousness. Code author should take the feedback in with an open mind. It's hard to receive critique of ones own code. It's hard to distinguish personal preference from best practice. This can be solved with mutual respect, trust and good intentions from both parties. With that, benefits of a quality code review span far beyond a single pull request.

If you have any feedback on the subject of code reviews, you can reach me: