Friday, January 27, 2012

Coding Good Comments: Exhortations

Another kind of comment inside program code is the Exhortation: A warning or reminder for the maintenance programmer when making changes to code in the vicinity.

Exhortations may be the only kind of comment that deserves to be decorated with boxes, ALL-CAPS and/or exclamation! marks because they are the only kind of comment that may be more important than the surrounding program code... by definition all other comments are less important than the program code because

  • comments aren't compiled,

  • comments aren't tested,

  • comments aren't executed, so therefore

  • comments can easily be (and often are) wrong.
Exhortations are a bit different: they offer blunt warnings, not subtle guidance or explanations. When an exhortation becomes out-of-date, and therefore wrong, it's usually easy to detect and easy to resolve by simply deleting the comment.

When an exhortation is valid, however, it is often valuable to both newcomers (as a warning about a possible pitfall) and experienced programmers (as a reminder about that pitfall).

Example 1

Here's an example from inside a SQL script that contains CREATE TABLE and other definitions for tables that are subject to the "Data Upgrade" process when an old copy of the application database is upgraded to a new version:
-- ***********************************************************************
-- ***** EVERY CHANGE DOCUMENTED HERE MUST ALSO BE DOCUMENTED IN     *****
-- ***** 015c_rroad_data_upgrade_startup.sql EVEN IF IT IS           *****
-- ***** NOT SIGNIFICANT TO THE DATA UPGRADE PROCESS.                *****
-- ***********************************************************************

What it's saying is that every single schema change made in this module must be checked against code in the other module to see if anything has to be done over there as well.

The "MUST ALSO BE DOCUMENTED" part of the exhortation is saying that a record of that checking must be kept in the other module; in this case, the modification history comments are stored in both locations as evidence the change wasn't forgotten.

Example 2

Redundant code is sometimes just easier to deal with than to eliminate with complex logic. Here's an example of an exhortation that warns of the existence of a debugging version of a tble; when the original base table is altered, the debugging version probably needs to be changed as well:
   -- ********************************************************************************
   -- ***** WHEN MAKING CHANGES HERE, SEE ALSO THE TRACE_* VERSION OF THIS TABLE *****
   -- ********************************************************************************

Example 3

Here's an warning about NULL-versus-non-NULL values that was added to the code after several catastrophic mistakes were made; this warning may only benefit newcomers, but it doesn't hurt for experienced developers to be reminded as well:
   -- *****************************************************************************************************
   -- ***** DO NOT CHANGE NULL / NOT NULL CONSTRAINTS WITHOUT CHECKING EVERYWHERE THE VALUES ARE USED *****
   -- *****************************************************************************************************

This exhortation appears twice in the code, and so does the one from Example 2: both at the beginning and end of the associated CREATE TABLE statement to reduce the chances it'll be missed.

Example 4

Here's an example involving the same business rule being implemented by separate UPDATE statements:
         -- ***** CAUTION ****************************************************************************
         -- *****    When changing the UPDATE statement here, also check the similar UPDATE in   *****
         -- *****    204_rroad_monitor_sample.sql.                                               *****
         -- ******************************************************************************************

The business rule's the same, but the syntax and semantics of the UPDATE statements are different, and the logic required to implement a single common UPDATE was deemed not worth the effort, not when the changes are easy to make in two locations and an exhortation takes care of the "oops I missed that" problem.

Example 5

Here's an example of two alternate versions of the same table, only one of which exists at run time but both must be maintained... and most changes must be made to both versions.

Once again, the exhortation appears twice inside each table because they're quite long; the ellipsis '...' represents many columns.
IF '{DEBUG_MESSAGES}' = 'ON' THEN

   -- Include all constraints to facilitate development testing.

   --------------------------------------------------------------------------------------------
   -- CAUTION: If any changes are made here, make the same changes to the other CREATE TABLE. 
   --------------------------------------------------------------------------------------------

   CREATE TABLE rroad_sampling_options (

      sampling_id                          UNSIGNED INTEGER NOT NULL DEFAULT AUTOINCREMENT,

      ...

      --------------------------------------------------------------------------------------------
      -- CAUTION: If any changes are made here, make the same changes to the other CREATE TABLE. 
      --------------------------------------------------------------------------------------------

      PRIMARY KEY ( sampling_id ),
      UNIQUE ( selected_tab, 
               selected_name ),
      CONSTRAINT "NOT ( sampling_should_be_running = Y AND timed_out = Y )" CHECK ( NOT ( sampling_should_be_running = 'Y' AND timed_out = 'Y' ) ),
      CONSTRAINT "last_sample_started_at <= last_sample_finished_at" CHECK ( last_sample_started_at <= last_sample_finished_at ),
      CONSTRAINT "last_canarian_query_started_at <= last_canarian_query_finished_at" CHECK ( last_canarian_query_started_at <= last_canarian_query_finished_at ) );

ELSE

   -- Omit some constraints to improve performance.
   -- Not all constraints are omitted, just the ones that might affect performance.

   --------------------------------------------------------------------------------------------
   -- CAUTION: If any changes are made here, make the same changes to the other CREATE TABLE. 
   --------------------------------------------------------------------------------------------

   CREATE TABLE rroad_sampling_options (

      sampling_id                          UNSIGNED INTEGER NOT NULL DEFAULT AUTOINCREMENT,
      ...

      --------------------------------------------------------------------------------------------
      -- CAUTION: If any changes are made here, make the same changes to the other CREATE TABLE. 
      --------------------------------------------------------------------------------------------

      PRIMARY KEY ( sampling_id ),
      UNIQUE ( selected_tab, 
               selected_name ) );

END IF;

The Bottom Line

Exhortations should be blunt, simple and to the point, and if at all possible they should be positive: They should tell the developer what to do, not what to avoid.

No comments: