Code Reviews, just better

“Code Reviews Are More Important Than TDD”

Dr. Venkat Subramaniam

Bildschirmfoto 2014-01-06 um 15.42.40Don’t get me wrong: Test Driven Development is great and it helps with a lot of things: Good quality, maintainable functionality, repeatable tests and much more. We did a survey at conferences that showed us that around 70% of all developer teams are practicing some kind of TDD but just 30% are doing regularly code reviews. Why is that? Here are some explanations why test automation is so much more common:

  • It’s automated: Developers love to automate things. You can use a fancy build server and show that your code is running.
  • It’s an easy to follow process: You write tests, you write the code, you run the tests. Easy to follow for each individual programmer.
  • You do it on your own: There are no dependencies: Developer don’t have to wait for other developers to write tests, no social interaction is needed, the machine tells you if your code works or not.

The benefits of Test Driven Development are obvious, but here is why I think code reviews are more important:

  • Better code quality: Reviewing the code by a team member can maybe not catch all functional bugs but it can catch obvious mistakes, correct bad variable, method and class naming and it improves the readability of the code (and we all know that we read code way more often than we write it).
  • Learning: When reviewing code developer can learn from each other to write better code. Even intern can give senior devs a new perspective. This is also recommended practice for book writers: Read! But it also helps to understand the whole code base and not only the parts you are normally working with. You will review code from other parts of the system that you maybe were afraid to touch yet.
  • Feeling better: Having another developer looking over your code before it goes into production can make you feel better. You don’t feel that you’re the only one responsible for a potential bug. There are others that haven’t seen it either!

Code Reviews with less meetings & better tools

So what’s the problem with code reviews? Why are not all teams doing them? First we should all agree that the time reviewing code is well spend! Some teams practicing a meeting every week to look at the code written the last 5 days or at a special feature. These can be a painful sessions for various reasons.

Make reviews less painful with tools.

Closeup of human hands pointing towards business manWhen we discuss code in front of the whole team it can be hard for the author: Everybody is staring at his code and give comments. I’m not saying that it can’t work, but here we have the same social problems as we see with Pair Programming (and there’s just one guy looking at your code). I’ve seen people not showing up or coming with stomach problems to work because his/her code was on the schedule for the code review meeting that day.

A tool like Gerrit, GitHub, BitbucketStash or Crucible can help because the author has more time to about the feedback and don’t feel like going into defense mode right away. In code review tools authors can invite other developers to participate in the online review. You see the code changes and add comments to the code. Developers with strong opinions won’t take over the review in tools that easily and others that may be a bit more shy have a lower barrier to participate in the review. But the same rules apply also for online reviews: Be gentle!

Make the review part of your process.

stash1.3_threadCom_WhatsNew288Don’t make code reviews an end-of-the-project event. Reviews can get long and boring. Keep them small. For people that already uses Git with feature branches this one is easy: If you want to merge changes back to the main branch you can create a Pull Request. Git tools like Bitbucket, Github or Stash have a visual interface to make comments. No change will touch the main branch without a review (if you want). Just make it a natural part of your process: If you have a change, it needs to be reviewed. Keep changes small so people are not afraid of reviews. A task is not done if the connected code review is still open.

Some tools also offer the ability to see the code changes in context. What’s the problem these changes should solve? Most code review tools let you connect to an issue tracker to be able to jump from the review directly to the issue.

If you still prefer review meetings do them regularly, so people get used to them. I think once a day would be best but at least two days a week. Never cancel the meeting because of too much work and try to review every change. If you can’t run through all changes split the group and rotate the group members.

Review code when it fits into your schedule.

2339687721_67d1d5146eThat’s a problem with another meeting on our schedule: They never fit into our personal flow. If you do code reviews in tools you can do them whenever you got time: In between programming tasks, first thing in the morning or after the lunch break; independent from team mates and their schedule. Also: Don’t expect every reviewer to finish review your code. Someone will always not make it within one or two days or gets sick. Have a rule that you invite maybe 4 people and mark the review as finished when 2 people have approved the review. Just make sure that it’s not always the same people that are not reviewing the code.

Here you can find more tips for code reviews from Mattias Karlsson.

Photo Credit: Simone Lovati via Compfight cc
Photo Credit: Giorgio Montersino via Compfight cc

One thought on “Code Reviews, just better

  1. Couldn’t agree more on this. Code review prevents several hours of debugging, especially while chasing concurrency bugs, which has better chance to be spotted during code-review process. It’s also improves code quality and learning of both reviewer and coder. Win Win for all.

Leave a Reply

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

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

Facebook photo

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

Connecting to %s