www.gowland.ca

The following is the result of some lessons learned from code reviews done for me by peers.

Before Sending A Code Review

• Is there a chance that testing was done without updating all segments of the applications? (Eg. WebArch was not updated during testing.)
• Update repository and rerun all tests before making diff
• Is test data such that the result of the tests doesn't truly indicate pass or fail? (eg. all employees have the same MaxBlocksWanted)
• Are there potentially some uncommitted files? (svn status)
• Have all tests and/or validations been run?
• Use the rest of the checklist to do a self-review of your own code

General

• This completely addresses the given MKS unless the comment specifies it is a partial solution with more code pending
• Result of function calls captured, verified, and handled
• Variables' scopes are minimal
• Variables' names are clear
• Variables are used close to where they are declared
• Loop boundaries
• For non-C code, approximately adheres to C coding standards
• Follow failure paths through each function:
  • make sure all paths are handled
  • make sure error messages make sense for all paths that terminate there
• Sufficient comments?
• Sufficient logging?
• Is there any corresponding code that needs to be changed as well? (Eg. Change to a deserializer w/o a change to the corresponding serializer.)
• Are error messages easy to understand, well formed?
• For 'if' statements, are all conditions dealt with (especially if there is no 'else' clause)?

Using SQL in code

• Bind variables to correct fields of SQL statements
• Read fields of SQL selects to correct variables
• Are there any side-effects from JOINs?

C/C++

• Coding standards adhered to:
  • no () casting
• No unnecissary qualification of member functions in header (screws gcc4)
• Check that all exceptions are being caught
• No reference variables pointing to temporaries

C#

• Check that all exceptions are being caught
• Are all the "using" statements still necessary?

Perl

• strict/warning/-w used
• use perlcritic?
• String to int conversion w/o suspending warnings
• Variable name typos (should be checked by strict/warning/-w)
• Are variable assignments happening in the expected context?