Peer Review

Developers on my team have a self-imposed policy of peer reviewing every change we make. This adds some overhead to fixing problems. But the benefits far outweigh the costs. I continue to be surprised by the value added when a second pair of eyes checks my work.

There are 2 main documents generated by a developer for peer review: (1) Code Diff and (2) Unit Test Plan. The Code Diff tells a review what you changed and why you changed it. The Unit Test Plan is an outline of how you debugged and verified your changes. Sometimes the act of actually writing your unit tests down improve their quality.

Recently I was asked to peer review a change in the sorting for a spreadsheet on one of our screens. The sorting was multi-column, along with some non-trivial rules for the sort order. Luckily I had a lot of domain knowledge for this part of the app. Here are the things I looked for when doing the review:
  • Were the variable names chosen to provide meaning?
  • Are there any "magic numbers" in the code?
  • Are tricky pieces documented with meaningful comments?
  • Was the code written so that it could be easily maintained?

In the end I had to go through two passes of peer review with the developer who coded the changes. The implementation worked. But the first cut was a maintenance nightmare. I tried to focus on the work product and not the developer. And I made sure the atmosphere was truly that of a peer assisting with quality improvement.

In the end, I think the routine turned out to be a solid piece of code. Only time will tell for sure. But I bet a new guy could come in, read the code, and understand what is going on without ever talking to the original developer.