Other posts in the “Delivering high-quality software” series:

1. Delivering high-quality software – a clean code

2. Delivering high-quality software – a code review

A proper process of code review is another part required to deliver software that not only “works” but also evolves correctly. Doing a code review is like playing a good game – “easy to learn and difficult to master”. The are several challenges we have to face in this process, yet we may be unaware of them. Today I am going to write a little about these challenges and tips that can help to overcome them. At least they helped me:)

The purpose of the code review

First things first, the code review process is a must if the codebase has to evolve in the right direction. If the process is not established in the company you work for, go ahead and try to change it. Why this is so important? A code review is a great tool both for the reviewer and reviewee for various reasons. Here are a bunch of them:

• The code review makes the codebase improve over time and evolve in a good direction. This would be far more difficult without this process.
• We learn how the project evolves and what new API/solutions we can reuse.
• We learn new syntax, language features, and design patterns.
• We can also teach others.
• It is a great opportunity to discuss further technical solutions and the way they were implemented.
• Pull requests together with the code review comments and discussions are great documentation (if persisted, of course).
• We can praise each other!
• …but we also learn humility and how to detach mentally from our code. Getting our pampered code “criticized” is not that pleasant:(

What to look for while doing a code review?

While doing the code review, it is good to review at least:

Overall design of the solution

Ask these questions first:

• Is this change reasonable? Do we need this at all?
• Do we need this change in this particular microservice/module/other component? Are there any better places to put it?

If the change is reasonable and resides in a proper place, it is worth collapsing the tree of the change to understand the way classes are distributed within packages. Maybe this layout can also be tweaked?

Style and consistency with the existing codebase

Maintaining the same style and being consistent across the codebase is important. Automating this stuff can be a great relief and time-saver. You can use several tools to do the job for you, I have written more here: Automate your code quality in a few minutes.

Naming

Tough thing, as it is often difficult to come up with a meaningful yet short name. Code review is a great way to discuss the naming.

Implementation of the functionality and APIs

Is this feature going to work at all? Is it implemented reasonably? Last but not least – Will I use the new API with ease?
The introduced code has to be usable both for end-users and for programmers.

Documentation and comments

If the code is self-descriptive, we can omit that point. Good documentation can be a life-saver though. If documentation has been updated, it has to be also a subject of the code review. Comments are more tricky – too many will obfuscate the logic, but sometimes we need an additional explanation alongside the code.

Possible bugs and edge cases

What can go wrong with the current implementation? How the code handles exceptions? Is only the happy path covered? Can users “spoil” this solution? – we need also to ask these questions during the review.

Tests

Is the code tested properly? Are these unit/integration tests enough to cover all edge cases? Are they testing the code at all?:) Are tests well written with the same care as the production code? Last but not least – Can tests serve as documentation?

The structure of commits

The overall structure of commits and commit messages are often missed in code reviews, at least from what I noticed. I have written more about how to make things right here: Commit to the quality of your commits.

Challenges in code reviews

As I mentioned earlier, there are various challenges we can face in this process. Among many, I have listed a few of
them:

Lack of motivation

Developers often do not have sufficient motivation to perform code reviews well. There are various reasons but one of the most common is the lack of time. A good code review takes time and things are getting even worse when developers are flooded by multiple review requests at the same time. Not to mention they all have deadlines to deliver their implementations. In such scenarios, code tends to wait long before getting merged (causing additional frustration and lots of merge conflicts). Alternatively, it is just quickly scanned without a proper understanding – just to “merge and forget”.

Lack of context

It is difficult to do a code review well when we do not fully understand a system. I think we have to live with that from time to time and perform at least checks we can do – like related to overall syntax, naming, and language conventions.

“Baby syndrome”

We usually treat our code like our baby. It is sometimes difficult or even crushing to have our code criticized therefore we need to get the ability to detach from our code. Practice makes perfect. And remember – even if we can do it easily, it does not mean others can do it 🙂 Therefore, we have to be gentle in our comments.

Fear of reviewee: showing an incompetency

It is easy, especially for younger developers, to fear code reviews because it could potentially reveal their imaginary “incompetency”. I remember that as a junior developer I was even ashamed to look for solutions in “StackOverflow”:) It takes time to understand that we are always going to be somehow “incompetent” and that is OK. I have written more about all those “syndromes” here: Dev tips no. 4 – Know who you are.

Fear of reviewer: showing a weakness 🙂

The funny thing is that the reviewer can also fear showing his/her imaginary incompetency. What if I do not find an easily-visible bug? What if my comments do not make sense at all?

Too big pull requests

Ask a programmer to review 10 lines of code, he’ll find 10 issues. Ask him to do 500 lines and he’ll say it looks good.

— Giray Özil

Code reviews shall be quick and big pull requests do not help in this matter. It is always better to split the feature into several parts and create multiple small pull requests instead of a gigantic one.

Too slow process

I was working once for a company that paid attention to code reviews and these reviews were just great. Developers looked deeply through the code, left very helpful and meaningful comments and there was a great feeling of mutual respect. Still, the process was so slow – it often took weeks for the pull request to get merged. This was frustrating as the codebase was changing often. I had to resolve conflicts multiple times before the code was eventually accepted.

How can we make things better?

Start from tests!

To understand the bigger picture of the change quickly (and to check tests quality at the same time), start the review from tests. You will understand the overall use of introduced/changed API, data structures, and so on. Tests shall be an integral part of the change and shall not be pushed later, as a separate pull request.

Start from the bigger picture

Look first at the package structure and overall layout of classes. Even by a simple glance, it may be possible to understand how things are connected.

Things get simpler in the IDE

Sometimes it is worth checking the reviewed branch out and starting analyzing the code directly in IDE – especially for bigger pull requests. Invest a minute to import the code to speed the whole review up.

Do not interrupt your work by reviewing the code

Unless there is an emergency, finish your work first (or at least a part of your work) and then start reviewing the peer’s code. It will allow you to fully focus on the code review without the unnecessary context switch back and forth.
It is also beneficial for your implementation.

Put a screenshot of UI changes in pull requests

Taking a print screen of the UI change and pasting it to the code review tool takes only a few seconds.

Small PRs

As I mentioned earlier, make rather small pull requests and demand that also from other developers 🙂

Think about badges

Your team can agree on badges placed in front of the comment indicating how to proceed with the comment, like:

• NIT – nothing important, the developer may or may not apply the suggestion
• TIP – a tip for the future, nothing to change now
• NT – something that can be changed in the next PR, etc.
• BUG – a potential bug
• … and so on.

One-day rule

My golden rule is to review the pull request no later than the next business day. A code that waits too long for a review gets outdated quickly and the creator already switched his context to the next feature.

If you cannot understand the feature, focus just on the code

Sometimes it is not possible for us to fully and quickly understand the feature to review. It is good to say this aloud informing the reviewee that we can check the solution in terms of the syntax, API, conventions but we are not capable to find any more subtle flaws this time.

A mediator can be helpful

If there is a prolonged discussion between a reviewer and reviewee and neither wants to agree, it is beneficial to bring one another person to the table. The person will work as a mediator who will help to agree on a final solution.

Be gentle

Remember – be always as gentle as possible. Always suggest, do not criticize.

Give additional guidance

Additional information (like a link to 3rd party site, documentation, etc.) is always beneficial if placed in your comments. It is a great opportunity to learn, teach and document our solution.

Praise the code!

Comment also pieces of code you like – and do it often! Even a few kind words can make the overall code review experience so much better!

Summary

While the idea looks pretty simple, it requires time, effort, and practice to make good code reviews. Lack of time, a proper understanding of the code-base, and sometimes… soft skills make this process unpleasant:( With a proper approach and care, code reviews can be fun and bring the possibility to teach and learn.