A code review is when a developer asks one or more of the other developers on their team to give them feedback on changes to the codebase. It can be a formalised part of the process or an ad-hoc activity, like pulling a co-worker over to your desk to rubber duck through a problem.
Like all additions to your process, code reviews should pay rent. In other words, they should be removed from your process unless they (at least attempt to) provide you with measurable benefits.
Code reviews lead to the following benefits:
Confidence and Shared Responsibility
By reviewing and accepting your code, other developers are giving their approval of your implementation. Potential issues with implementation are highlighted and can be fixed immediately rather than in a future refactor.
Getting your head stuck into the fine details of a problem can sometimes make it difficult to spot glaring errors in your implementation. Having at least one other developer do a quick scan of your code will help pick off any low-hanging fruit you may have missed writing it.
There are however a few studies† that indicate that code reviews highlight improvements in evolvability rather than correctness of code. Quoting the linked paper, 'evolvability' includes classes of error such as "long methods, duplicate code or incorrectly located functionality".
You might measure this benefit by keeping track of reported errors in delivered software before and after starting to use code reviews as part of your process.
More Learning and Discussion
Discussing issues like reusability, separation of concerns and testing around concrete examples rather than in the abstract has two benefits.
Firstly, it allows developers to learn from each other with real-life code that is immediately relevant.
Secondly, it improves the quality of the project you're working on right now and helps developers converge on shared values and norms.
Some examples of shared values:
- DRY (Don't Repeat Yourself)
- Composition over inheritance (a.k.a. 'Separation of Concerns')
Shared norms might be:
- Expose RESTful web services instead of allowing direct database access
- Blacklist request parameters instead of whitelist
- Break domain logic into separate libraries where possible
- Regularly update libraries to newest versions
There may not be a way to quantitively measure this benefit. At a stretch, you could poll developers to ask how 'cohesive' the team feels on an arbitrary scale (say 1-10), and test if the average goes up after starting code reviews (given the same amount of time without code reviews beforehand).
Especially on larger projects, it can sometimes be difficult to keep up with exactly what is being implemented right now. By participating in code reviews across your company/codebase, you can pick up the details of other projects through osmosis.
Again, aside from some sort of self-reported feeling about knowledge of the project, there probably isn't a way to quantitively measure this benefit.
Pull-requests as code reviews
Github pull-requests make starting code-reviews easy. As Github provides an interface allowing you to browse code and comment on changes, the entire code review can be as simple as looking over changes in a proposed merge, accepting suggestions and subsequent changes and then merging when the changeset is ready.
Ad-hoc Code Reviews
Instead of imposing code reviews from the outside, I've found that giving developers the option of having their code reviewed is sufficient for getting all of the benefits outlined above.
If a developer working on implementing a feature or fixing a bug desires feedback on his solution he can request a review, fully aware of the cognitive load and time the reviews will cost other developers on the team. The same developer could choose not to request a review if the changes are trivial.
Overall, working on teams using some notion of peer-review tends to be a much smoother experience than otherwise. I've found that there's a greater sense of camaraderie and ownership over the implementation that results in fewer errors and higher code quality.
- Najaf Ali