Active and reactive code review

Posted by Jim Jagielski on Friday, November 10. 2006 in Programming

For any software project, continual code review is crucial. In general, there are 2 methods, at least regarding how I do it. The first is what I call active review. That's when I take time out and pick out a code segment at (semi) random and just read it. I follow any function or method calls and track how the code works. Is it logical? Is it robust? What improvements can be made? I usually find at least some way to make it better. The second is reactive and it generally occurs either when tracking down a bug, or when evaluating a patch. I follow the same procedure as with active, but the starting point is obviously different. The cool thing is that most of the time, in addition to fixing the bug (or validating the patch), I almost always discover something which can be improved or fixed. A good example of this just happened: we're looking at possibly doing a release of Apache HTTPD 2.2.4 in the very near future, and one potential backport was being discussed. This got me looking not only at the backport code itself, but the other functions that the code uses. This lead me to our implementation of apr_socket_recvfrom() in APR. Unlike the "real" recvfrom(), ours requires a from argument. In fact, if you look at some of the various test scripts, they have to go out of their way to create bogus from info to test the function. Why? Certainly it makes sense that one should allow a NULL from, and have apr_socket_recvfrom() do the right thing... posting this to the APR dev list indicated that other APR developers thought so as well, so I'm going to spend some time implementing that. Sometimes it's the unexpected byproducts of reviews that are the most beneficial.

The author does not allow comments to this entry


Search for an entry in IMO:

Did not find what you were looking for? Post a comment for an entry or contact us via email!