On Code Reviews

There seems to be an established truth in programming that code reviews find a lot of bugs. An example is chapter 18 (Modern Code Review) in the book Making Software. This is however not my experience – I rarely find outright bugs when reviewing changes. But I still think code reviews are useful. Below are my thoughts on the value of code reviews, and how to make the process efficient.

Very few bugs found. When I review code, it is quite rare that I find bugs. By bugs I mean cases where the program would do the wrong thing. I don’t count things like poor variable names, missing test cases, or misleading log messages. All those things should be corrected, but if the program ran, it would still produce the correct result. I find outright bugs maybe once or twice a year. Perhaps it is because I am bad at reviewing code. Or perhaps it is because I have really competent colleagues. Either way, it would be interesting to hear about other people’s experience in this respect.

Why code review? Even though I don’t find many bugs, it is still worthwhile. First of all, it is similar to proof reading text. It can be hard to spot your own mistakes. Therefore it is good with a second pair of eyes to check the code. Even if there aren’t any bugs, there can still be issues that should be corrected: unclear code, poor naming, missing tests etc. And once in a while there actually is a bug. Code reviews also help to spread the knowledge of how the code works.

Too late to find big problems. If I am developing a feature, and it is not obvious how or where to make the change, I like to discuss it with one or more people in the team before I start. Having a discussion like this before I start means that I will hopefully find out if there are better ways of doing it than I have thought of so far. Perhaps the change should be made somewhere else? Or maybe I could use an already existing feature I didn’t know about. If problems like these are only detected during code review, then I have wasted a lot of effort coding and testing that change.

Review, not test. Some people think that to do a proper review, you should pull down the changes and run and test the code. I think this takes too much time and effort. The original developer should test the code, otherwise how do they know that what they have developed works? Given that the code has already been tested, it is enough to review the code without also running and testing it.

Speed. One problem with code reviews is that they force you to context switch until the review is complete. The longer the review takes, the worse it is. In my team we always prioritized code reviews for this reason. You would write a message in the chat, and usually somebody in the team would start the review more or less immediately. In a team of ten people, there is usually someone that can jump in and do the code review without it interrupting their own work too much.

Ask for context. All code reviews are not created equal. Some are more complex and difficult. In those cases, it is good to talk to the developer before starting the review. They will be able to fill you in on why it is done the way it is. Sure, you can read the description in the ticket, and you can figure most things out from the code. But in my experience, it is much more efficient to first talk to the developer directly.

Criticism. When there are problems in the code, how you phrase your concerns is important. If there is a lot to complain about, I think it is nice to talk the developer if possible. First of all, you can check that you haven’t misunderstood what the change is supposed to do. If you haven’t, then it’s easier to not come across as too harsh if you have a conversation about it. But sometimes a conversation is not possible. Then I think it is good to be diplomatic in the comments. For example, instead of writing “This part should be in its own function”, you can write “Mabye this part should be in its own function?”. At least in my experience, the question-comment works quite well.

Praise. When you see something good in the change, don’t be afraid to add a comment about it. It’s nice to get some acknowledgement, since most comments only point out problems.

Conclusion

What is your experience? Do you find many bugs when doing code reviews? Let me know in the comments.

4 responses to “On Code Reviews

  1. We often find bugs in code reviews, but our team consists of fairly new developer. But even then maybe one bug in 30 code reviews.

    As some how has only worked with subversion prior to git, the code reviews have been an completely new experience. I love them for things like naming, code style and the generally thoughprocess „someone else WILL look over it, better make it right the first time“. You also know what happens in part of the code you have never touched and get a better understanding of the complete application, just because you have read everything (I don‘ necessarily understand all the code, but I at least have seen it).

    All in all, I think I would have real difficulty to go back to a version control without code reviews.

  2. Interesting research paper that makes the same observation that I do: even though bug finding is a top motivation for doing code reviews, it is not very common to find bugs. Instead, code improvements and knowledge sharing are the most common outcomes.

    Click to access ICSE202013-codereview.pdf

  3. Pingback: Java Weekly, Issue 416 | Baeldung

Leave a Reply to Byte Cancel reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s