Code Reviewing - Improving our process

We have implemented a code review process over the past 3 years which seems to work reasonably well. This helps considerably to reduce the number of defects QA and ultimately the customer will see. However it’s only recently that I’ve started to analyse our current process with a view to improve it.

One of my biggest concerns is that we find a lot of non functional defects and not many functional defects in code review. Whilst the non functional defects are important in terms of helping with code maintainability, functional defects should be the top priority on any code review as these defects usually manifest themselves as issues which would be visible to a customer.

I have recently read a copy of “Best Kept secrets of peer code review” which is freely available from smart bear software at http://smartbearsoftware.com. It provides useful techniques which can be used to improve your code review process and are based on real life case studies. Although of course the book does lean towards using smart bears code collaborator product to help with the code review process it does have a lot of useful content to warrant reading it even if you’re not likely to purchase these tools.

Some points I picked up from the book which I’m going to look at in improving our process are

1. Initial quick scan – The first initial scan can be very important in helping find defects throughout the code. As there is a negative correlation in the time the reviewers takes on the first scan versus the detected speed. The more time the reviewer takes on their first scan, the faster the reviewer will find defects. If you don’t perform a preliminary scan your first issues identified may focus on the wrong areas. With a quick scan you can identify areas with the highest possible issues and then address each one with a good change you’ve selected the right code to study.
2. Review for at most one hour at a time – The importance of reviewing in short time periods shouldn’t be underestimated. We should try and avoid large reviews and try and split them up into smaller reviews where possible. E.g. you could certainly split up reviews of unit test codes versus core code. Although of course this isn’t an excuse to submit code with known defects for review.
3. Omissions are hardest defects to find – It’s very easy to find issues for code you’re reviewing, it’s much harder to find defects in code which isn’t there. Having a review check list illeviates this issue some what to remind the reviewer to look for code which may not be present. Defects that aren’t associated with any checklist item should be scanned periodically. Usually there are categorical trends in your defects; turn each type of defect into a checklist item that would cause the reviewer to find it.
4. Defect density and author preparation – Providing some preparatory comments before code is submitted for review does seem to have an impact on defect density. The key point here is that by annotating certain areas of code, developers are forced to do some kind of self review of their code which obviously increases the quality. Although there is a danger that the developer by preparing could disable the reviewer’s ability for criticism i.e. he’s not thinking afresh. However usually by developers preparing these kind of comments makes them think through their logic and hence write better code.
5. Leverage the feedback you’re getting - Improve your ability and make yourself a better developer by reviewing the points you commonly receive. Code review points should be learned so you reduce the number of defects over time. A useful exercise could be to put your comments in your own personal check list which you visit when ever performing a code review. Any points which aren’t covered on the global check list should be added
6. Developer and reviewer joint responsibility – Defects found in code review are good. A defect found in review is a defect which never gets to QA or the customer. Developer and reviewer are a team and together plan to produce excellent code in all respects. The back and forth of coding and finding defects is not one developer punishing another, it’s simply a process in which two people can develop software of far greater quality than either could do single-handedly. reviewer finds in the developers code the better, this should be thought excellent summary
7. Checklist – Use check lists as an aid to identify common defects. The most effective way to build and maintain the checklist is to match defects found during review to the associated checklist item. Items that turn up many defects should be kept. Defects that aren’t associated with any checklist item should be scanned periodically. Usually there are categorical trends in your defects; turn each type of defect into a checklist item that would cause the developer to find them next time.