[Insight-developers] Code review

Will Schroeder will.schroeder@kitware.com
Tue, 25 Sep 2001 10:38:56 -0400


Hi Folks-

Jim Miller and I went over the code again yesterday. Jim will be updating the excel spreadsheet with comments, and sending his own email at some point. I'd like to add some more high level comments about style:

  + people use tabs; please don't
  + people don't indent two spaces; please indent two spaces
  + people don't use curly braces {} to delimit loops and conditionals
  + the lead curly brace should be on the next line, indented in two spaces 
     (aligned with the code which is also indented two spaces)
  + Instance variables are not being spelled out; e.g. MaxNumIter is MaximumNumberOfIterations
      or NumClasses vs. NumberOfClasses
  + Instance variable names are spelled wrong; e.g., ErrorTollerance vs. ErrorTolerance
  + protected operator= and const constructor are missing
  + if you have a Get() method (GetNumberOfClasses()), then the print self should print the value
  + std::cout is still being used, use itkDebugMacro (requires "this->" pointer) or itkGenericOutputMacro (no "this->" pointer required)

These may seem petty, but for the user of the software style consistency is vital. Otherwise they will never know what a particular method or ivar name is without doing a documentation search; meaning slow and aggravating. The curly brace style is important to indicate which code is associated with a particular conditional, etc.

I've made some changes as I go through the code; it will probably break some of your internal applications.

Will