Visual Basic —

Code Reviews

The code review (or inspection) process is a major part of the software quality cycle, and it is also one of the most important. It is an acknowledgment that the creation of test scripts or the use of automated testing packages only goes so far in assuring the quality of the code. Computers do not yet possess the levels of reasoning necessary to look at a piece of code and deduce that it is not necessarily producing the result specified by the design document. I guess when that day comes, we'll all be out of a job.

The code review is the process whereby the human mind reads, analyzes, and evaluates computer code, assessing the code in its own right instead of running it to see what the outcome is. It is, as the name suggests, a thorough examination of two elements:

  • The code itself
  • The flow of the code

A code review should also ascertain whether the coding style used by the developer violates whatever in-house standards might have been set (while making allowances for personal programming styles). On a fairly large project a review should probably be conducted twice. The first review should be early on in the development, for example when the first few substantial pieces of code have been written. This will allow any bad practices to be nipped in the bud before they become too widespread. A subsequent review much later in the development cycle should then be used to verify that the code meets the design criteria.

The value of this process should not be taken lightly-it's a very reliable means of eliminating defects in code. As with anything, you should start by inspecting your own code and considering what the reviewer is going to be looking for. The sorts of questions that should come up are along these lines:

  • Has the design requirement been met?
  • Does it conform to in-house development standards?
  • Does the code check for invalid or unreasonable parameters (for example, a negative age in a customer record)?
  • Is the code Year 2000 compliant?
  • Are all handles to resources being closed properly?
  • If a routine has an early Exit subroutine or function call, is everything tidied up before it leaves? For example, an RDO handle could still be open. (The current versions of Windows are much better than their predecessors were at tidying up resources, but it's still sloppy programming not to close a resource when you are done with it.)
  • Are all function return codes being checked? If not, what is the point of the function being a function instead of a subroutine?
  • Is the code commented sufficiently?
  • Are Debug.Assert statements used to their best advantage? We've been waiting a long time for this, so let's use it now that we have it.
  • Are there any visible suggestions that infinite loops can occur? (Look for such dangerous constructs as Do While True.)
  • Is the same variable used for different tasks within the same procedure?
  • Are algorithms as efficient as possible?