Monday, April 04, 2005

Pretty is as Pretty does



Click here for AmazonI saw an interesting blurb in Software Development Magazine in which developer John Elrick espouses the benefits of comment-free code. His sidebar article, a response to Christopher Seiwald's "Pillars of Pretty Code" posits that comment blocks are "crutches"; especially when compared to short, cryptic variable names that could be longer and more explanatory.

He provides sample code to illustrate his point.

Status uNlinkRec(Record **listHead, Record *const recordToRemove) {
  Record *currentRecord, *previousRecord = NULL;
  previousRecord = *listHead;
  for (currentRecord = *listHead; currentRecord; currentRecord = currentRecord->next) {
    if (currentRecord == recordToRemove) {
      previousRecord->next = currentRecord->next;
      currentRecord->next = NULL;
      return OK;
    }
    previousRecord = currentRecord;
  }
  return ERR;
}


I don't have a problem with developers omitting comment blocks (occasionally). First off, I would agree with John that effusive variable names should be required... especially in any language (C, C++, Java) that doesn't incur any performance penalty for long names (another reason fast typing makes a difference! Slow typists are usually loathe to use long variable names... :-).

But there is no question that, under most circumstances, comment blocks help! Few code snippets exist in sanitized, easily digestible modules like the one John used, above. Consider the following production code:

  //  Does first JPG chart exist and is up-to-date? If not,
  //    write a new one.
  //
  bXLS = FALSE;
  if (fileFind.FindFile(strFile)) {
    fileFind.FindNextFile();
    fileFind.GetLastWriteTime(timeXLS);
    bXLS = TRUE;
  }
  bHTM = FALSE;
  strFileOut = strFilePrefix + "1.jpg";
  if (fileFind.FindFile(strFileOut)) {
    fileFind.FindNextFile();
    fileFind.GetLastWriteTime(timeHTM);
    bHTM = TRUE;
  }
  //
  if (bFreshUpdate) {
    bGenerateCharts = TRUE;
  } else if (!(bXLS && bHTM && timeHTM >= timeXLS)) {
    bGenerateCharts = TRUE;
  }


Without the comment block, it would require some analysis on the part of the reader to figure out what was going on: in this case, the system is trying to determine whether a JPEG chart exists and is up-to-date (if not, a new one must be generated).

Variables that are long and descriptive are always preferred over short, cryptic names. But comment blocks should also be used whenever there's any doubt of the intent of the code.
 

No comments: