Image: FreeDigitalPhotos.net

I wrote this a few months ago when I was doing a lot of code reviewing. There are a number of criticisms of the Code Review, and certainly, a number of review mechanisms that can and should be used in conjunction with some type of human review, such as static code analysis and performance analysis. In this post, I won’t be comparing or contrasting the Code Review to any of these.

Strategies, Questions to Ask and Things to look for

Some code review questions…

• Is it relatively clear what the code does from reading it?
• Is the code in a reasonable location and named appropriately?
• Is this same functionality duplicated elsewhere?
• Are there any unnecessary calls, methods, variables?
• Do any of the url params have the potential to conflict?
• Has all text been pulled out into properties files?
• Would it be useful if this code could be disabled or enabled? If so, has the author provided a means for turning it off?

Tools

My experience with code reviewing tools is limited. The Jupiter eclipse plugin seemed to require too much work on the developers part—generating the request for review, checking in the .jupiter file and review files slowed development down. Monitoring svn check-ins seemed to work moderately well, although it required more work on the reviewer end.

• jupiter
• Review Board
• trac
• Crucible

When to do the code review

Ideally, code reviews shouldn’t uncover any larger architectural issues that need addressing. They should expose overlooking coding mistakes. So, ideally there should be no timing problem, that is, you shouldn’t find some large refactor that needs to be done before the code is released. Code should be reviewed after development is finished, but before the functionality is ready for qa.

Double Time

There are several different ways that code reviews can go: the code reviewer(s) can monitor check-ins, developers can request that code reviews be done on their code or the dev team can get together as a group and go over the code line by line etc etc. One thing that has worked for me is employing more than one of these methods. So, developers submit requests for review and I also monitor check-ins.