Wednesday, February 02, 2005

Peer Code Reviews

Even though I have had 8 years experience in the IT industry, it may or may not surprise you to know that until my current job I have never had a formal peer code review. In the past the only times that a peer has looked at my code has been in a very informal way, and not really looking for the kind of things that should be looked for in a proper peer code review. So as you might guess, I was a little nervous when faced with the prospect of a peer code review, however, I am now completely sold on the idea.

I believe there are 2 elements that make code reviews invaluable to good code production. Firstly, the reviewer has a different perspective on the code. The reviewer is someone with different experiences, and as they are not in the thick of the coding, have time to relax and view the code in a more objective way than the creator of the code. They are not subject to the same line of logic that the coder has gone through to produce the code, so if the coder has written something a certain way after a string of incorrect or sub-optimal logic, the reviewer is free to question the logic, or propose a better way of implementing an algorithm, and the coder then is forced to either justify his logic by tracing through his steps, validating his decisions at each point, or accept the better way and re-code accordingly. The reviewer can also check for consistency with the accepted coding standards, and if the coding standards are good, better adherence will result in more readable, more maintainable code.

The second element is the psychological factor. As a developer, if I know my code is going to be reviewed, I will be very careful to adhere to the accepted coding standards and I will make sure that my code is set out in such a way that it is easy for another person to read. I will also validate my logic, imagining how I will explain complex bits of code to my potential reviewer. And finally I will do a once over of my code before I ask someone to review it, tidying everything up, making sure the review process will go smoothly and there will be no serious issues that the reviewer could potentially pull me up on.

All this doesn’t guarantee bug free code, in fact, the purpose of a code review is not to replace a proper QA process, but it creates an opportunity to catch obvious coding errors that would have otherwise come up in the QA process, and would have extended the whole phase unnecessarily. As the QA cycle is always the largest unknown of any software project, anything that can potentially decrease this phase has to be a good thing.

The only thing I don’t quite agree with is the way in which they use their source control in conjunction with the peer code reviews. Their idea is that there should not be a single line of code under source control that has not been reviewed. This creates an obstacle to checking in regularly. My personal opinion is that there should be very few obstacles to checking in modified source code. At my last work the rules I instated were just the obvious two; only check-in if the project is building (ie don’t break the build), and check-in regularly (usually before you go home at night). I always get nervous if source files are checked out for too long, there are obvious problems that can occur. The issue is that to avoid large amounts of small code reviews developers will wait for multiple days until they have some serious code to review. I think that all though the concept is noble, maybe a similar concept could be achieved by having some simple processes around labeling (or tagging depending on the notation of your source control), that can be administered by the release manager. I'll have to see how the process goes over the next few months, and see if my opinion changes with respects to their source control methodology. One things for sure I'm a convert to peer code reviews.

No comments:

Post a Comment