Friday, September 11, 2009

Why you want code reviews

Lately there has been some discussion in our team on whether or not code reviews are needed and even practical and why you should want them in your development team. This may sound like obvious knowledge, but unfortunately it appears that some developers have an aversion against code reviews, whether it being their code being reviewed or it being them reviewing someone else's code. In this article we'll look into some of the reasons that makes developers feel this way and also we look into some of the benefits of having code reviews. As we go through all this, we will also give some attention at how to prevent bad experiences with code reviews.

Why not?
As I mentioned in the introduction, there is still a fairly large group of developers who have an aversion against code reviews. A lot of the time, this is because they feel that they did the best job they could on the code they have build, given the circumstances and they don't need some coworker grinding their hard labor to peaces and giving them a hard time.

If that's the case, something is obviously wrong. If this is feeling is based on previous experience, then they have experienced a bad code review by some reviewer who clearly missed the point.
Others may not want to be at that end of the table where they have to tell a coworker they did a bad job. Again, if you feel like this, you missed the point.
It is NEVER the goal to find something to be able to accuse the developer of bad work.

A third reason why a developer may not want code reviews is that they think it will be boring to do a review. If that's you, you guessed it, you've missed the point as well.

The goals of a code review
So what are we trying to accomplish trough a code review? There are several very important goals one could target:
  1. Code quality
  2. Code consistency
  3. Architecture consistency
  4. Awareness
  5. Knowledge sharing
Code quality
This may seem like the obvious first reason and causes the most aversion. Sure a developer tries to do the best job he or she can, however we are all human so we make mistakes. The goal here is not to bang on someones bad peaces of code, but to prevent bugs from happening. Bugs cause a lot of work for a software company. If they get catched by the testing department, they have to generate a case file for it and communicate that back to the developers, who in turn have to do rework and system test that. It then gets handed back to testing and they have to retest.
If a bug is discovered by a user, things get even worse. They will file a case with the support desk, who has to triage it and then it has to be planned for a release and generates a whole bunch of work. So in the end it is in everyones best interest to prevent bugs from happening, which is exactly what the first goal of a code review is all about.

Code consistency
To keep code maintainable it is imperative that chosen solutions are consistent with each other. However it is hard to know everything about a large code base that keeps expanding and changing while a team of developers is working on it. By having code reviews in place, a large part of inconsistency can be catched and fixed before the code actually gets to the testing department.

Architecture consistency
From experience I know it is hard to keep the architecture that was planned consistent throughout the system. Developers often tend to do things their way, especially when they either have difficulties understanding the architecture or how to solve a particular problem in the architecture, or because they run into time constraints.
To make sure that architecture at least stays consistent throughout the system, it is important to look at how certain functionality is built into the system. If a developer did something that affects the consistency, but brings a good argument for doing it that way, it may result in changes in the overall architecture because then changes are the architecture wasn't fitting the application. Again this is important to the whole team as some others may end up feeling the same pains the first developer encountered if it doesn't get addressed.

This may sound like a very generic term, so let me explain. Because we work in a rapidly changing field there is no way to keep up with everything that's going on so at times we may miss certain information which would have helped us solving a particular problem a lot easier or more efficient or prevent some problem in the future. To keep us more up to speed a reviewer may spot some of these issues and point out other possible solutions to a problem of which we might not have been aware.
Looking at this from a reviewers point of view you might actually see some features you where not aware of.

Knowledge sharing
This is a more obvious benefit. Not only does the developer get a change learn from the reviewer, but it also works the other way around. And this doesn't only apply to technical knowledge, it also goes for specific knowledge about the application that's being reviewed, which is an important business benefit.

Tips on code reviews
To make code reviews be a pleasant experience both the developer and the review need to keep an open mind. That doesn't mean you can't defend something you think is good work, or you can't say it when you see something you feel should be improved.

As a developer look at this as an opportunity to showcase your code to one of your peers. Reviewers, ask questions. They allow the developer to explain choices he or she made in the process. Both parties shouldn't be pushing something without a good explanation. If you can't come to an agreement on an issue, involve a third party you both trust and stick to that decision. Also don't take it personal, opinions can differ and there are more ways then one to reach a goal.

To take full advantage of code reviews, there should be a group of reviewers, who should be experienced developers in general and in the technology used. Also make sure the right reviewers are pared with the right developers. It's likely that a young but bright developer could review a more senior developer and technically this may make sense. However in the 'food chain' this might cause problems, so it's better to avoid this. On the other end of the spectrum a young developer with less experience on the technology used might be intimidated by a review from the most senior developers on the team. This might lead to the young developer not speaking his or her mind and just following the senior developers. Although this might get the junior developer a long way, it will not teach him or her to have a mind of their own.
For educational purposes it would be best to have a medior developer review a junior developer and have senior developers review medior developers. This way junior developers will be less intimidated and medior developers can learn the review skills from their senior peers.

I agree that there is a lot to think about when implementing code reviews, however I do feel it is worth the effort. A team doing good code reviews will get great benefit from them. Consider it and if you feel like it, please share your experiences with us.

No comments:

Post a Comment