Improving code quality using code reviews

Working in teams brings several challenges, one of them is a mixture of coding styles, causing code inconsistency across your project which makes it hard to read or follow what it does.

The bigger the team, the more important it is to transfer your knowledge about what you’ve worked on, so you have a limited Bus-factor in your team.

Or have you ever been working on a new feature fully confident that it’s ready to ship, only to notice that you’ve forgotten to take into account about the caching? Or that you’ve forgotten to update the documentation (if any)?

Sounds familiar to you?

Note – While some of these “pain points” can be tackled by using tools like Roslyn Analyzers or FxCop I prefer a more humane approach and discuss the why instead of the how.

Code reviews to the rescue

By using code reviews we can avoid these problems by collaborating before we ship the code – Let’s first take a look at an example:

While this code could perfectly process your order, there are some issues:

• Variable o represents a certain amount of state but what does it represent? Looking at the signature it is clearly an Order, but how do I know that in a 50-line method at the bottom?
• Time zones, they are evil! What happens if this code runs in the U.S.?
• Calling MyMethod<T> takes in a boolean but what does it really do and how does the boolean come in to play?
• How does the caller know what the Process method does? Hopefully bill for the order? Also, it couldn’t hurt to add additional documentation throughout the implementation in certain cases.

While performing a code review, the reviewer can indicate these concerns with the reviewee and have a polite & constructive discussion, backed by a set of coding guidelines. By doing this, both parties get to know how the other person thinks about it and they learn from each other. Yet, they also learn to express how they did something or what they have forgotten about.

Having a second pair of eyes on a certain topic can help a lot. Everybody has a different perspective and this can make sure that you forget about a certain topic and also, potentially, leads to interesting discussions. This gives you a certain “Don’t worry, I’ve got your back” feeling and forces you to think deeper about what you’ve written.

At the end of the review, the reviewee can have some feedback to process, where after the code gets the seal of approval and is ready to ship.

Next to the code quality, you also perform small knowledge transfers with each other. You will not remember everything but when needed you will remember certain pieces that can help you guide to the potential bug or cause.

Last but not least is automated testing. It’s a good thing to add the unit/behavior/scenario testing to your reviews as well because then the reviewer gets an indication of what you are testing and what you are NOT testing. Do the tests make sense or should the reviewee cover additional scenarios?

Challenges

Using code reviews is of course not a free lunch and it comes with its own difficulties.

The biggest challenge is that your team members need to be open for feedback and willing to incorporate the feedback! If they are not up for it, you will just spend your valuable time to only notice that they are ignoring it. You, as a team, will need to decide whether or not you want to commit to the code-review-system.

Every review takes a decent amount of time, so incorporate that into your planning. The reviewer needs to go through it, discuss it with the reviewee and then the reviewee need to process the feedback. However, one might argue that it is better to take your time during development instead of having to spend twice that amount while fixing bugs or trying to understand what’s going on.

New to code reviews? Here are some tips!

After using code reviews for a while, I’ve learned a couple of things on how to not do it or what can be challenging. Here are some tips that help you avoid some pitfalls.

Review early & frequently – The earlier you review, the better. This avoids reviewing something that is considered ready while you’ve misunderstood some aspects of it or have re-invented the wheel.

Define code guidelines – Agree upon a list of coding guidelines with your team to back your reviews. By doing this you have a clear list of styles, paradigms and DO’s & DON’T DO’s that the team should follow to unify your coding styles & practices. This makes reviewing a lot easier and a clear guidance on how it should look.

An example of this could be that each parameter should be checked for null and that it should throw an ArgumentNullException when appropriate.

Add code reviews to your definition-of-done – By adding code reviews to your definition of done you are certain that each new feature or bug fix has passed at least two pair of eyes and that multiple people agree on it.

By doing that, you also remove the burden of one person being responsible for one aspect, since it’s the whole team that is responsible for it.

Don’t review to bash, review to teach & improve – Finding a balance between being strict or agreeing with everything is hard. If you just bash it will have a negative impact on the team collaboration and frustrations will arise. Be constructive & open.

Review in-person but have a look at the changes in advance – This allows you to have a personal opinion instead of simply following the reviewee. This avoids you having to make decisions on the spot but instead digesting it first so you can think of obvious aspects

Challenge and be challenged – Ask questions about the topic to see if the reviewee has covered all the possible scenarios and learn about how they envision it. Discussions are a good thing, not a bad thing.

Learn from each other – Don’t be afraid to say what you like and don’t like or if you don’t know about something. Learn from others and how he/she did it this way and not like how you thought about it.

Conclusion

While the internet has a wide variety of blogs talking about this “concept” I wanted to share my vision on this since I’m a big fan of this practice and believe that this really improves the quality. However, your success will depend on the cooperation of your colleagues and project management if they want to commit in doing so.

One thing is certain – I’ve used this on my current project, but will keep on doing so in the future.

Thanks for reading,

Tom.