Principles of Code Comments

Everybody has opinions about comments in program code; here's mine:

• Don't include the file name in comments. Development tools always tell you what file you're editing, and it should be easy, not hard, to rename files when copying, reorganizing and so forth.

• From the moment in time the code in a new file is "finished" (from the ever-optimistic developer viewpoint), start recording modification history.

• Put the latest modification history comments at the top of the file, move old history comments to the bottom. You want to preserve history, but you don't want ancient history to get in the way.

• Keep the modification history simple. Don't write more than one line per modification history comment unless absolutely necessary. The future reader needs to know three things only: When the change was made, who made the change (initials), and what changed, where the "when" might include the build number as well as the date. It is perfectly OK to copy and paste a snippet of code unique to the change; future readers will take it from there to find the details.

• Put a "date started at" comment at the very top, and keep it there. Knowing when a file was originally created is very often interesting to the reader.

• Don't decorate comments. No big boxes, no government fill-in-the-forms.

• Do decorate warnings. It's even OK to shout, as in /* ----- WHEN MAKING CHANGES HERE ALWAYS CHECK FILE XYX.SQL ----- */

• Do include separator and title lines if the code's long and tedious... this assumes you agree one long flat module is easier to maintain than breaking it up into separate files simply to keep the line count down. However, if you would always rather maintain 100 ten-line files plus 100 calls, than a single thousand-line file, carry on!

• Don't clutter the code with embedded modification history comments, unless absolutely necessary. The "absolutely necessary" part might apply if, say, you are dealing with life-critical code, or for some other reason you need to communicate the exact details of the changes.

• Don't write comments that rot. Comments are not compiled, they don't have to be correct. Don't increase the reader's level of mistrust with details unlikely to survive the first maintenance cycle.

• Don't assume the reader is a n00b. If the reader *is* a n00b, it's up to them to learn the programming language, not you to teach them.

• Don't assume the reader knows all about the framework. Frameworks are not languages. Languages help, frameworks hide.

• Do include pointers to the docs. Not to the language Help, but to solution references and tutorials; e.g., where the original code came from, where relavent standards are found, and so on... if those are important to the code itself. URLs are great, Google search strings as well... the latter may survive link rot.

Here are some excerpts from a module that's at the very heart of the Foxhound Database Monitor; rroad_monitor_sample is called every 10 seconds for each target database to gather and save measurements about the server, database and client connections:

-- 2007 02 04 BC 1988a: Date begun.
-- (see bottom of file for earlier modification history)
-- 2009 07 16 BC 3440a: Changed: SET this_rroad_group_2_property_pivot.previous_ApproximateCPUTime = previous_rroad_group_2_property_pivot.ApproximateCPUTime,
-- 2009 07 16 BC 3440a: Added: Column temp_engine_properties.CurrentCacheSize       DEC(20),
-- 2009 07 16 BC 3440a: Added: Column temp_engine_properties.MaxCacheSize           DEC(20),
-- 2009 07 21 BC 3458a: Changed: ' Procedure rroad_ ... _properties not used; build number ', @procedure_build_number, 
-- 2009 07 21 BC 3458a: Changed: -- Also note: The ISNUMERIC call is used for the same reason: The proxy queries return duplicate garbled rows.
-- 2009 07 21 BC 3458a: Removed: Old commented-out code.
-- 2009 07 21 BC 3459a: Changed: SET @sample_elapsed_msec         = rroad_f_datediff_msec ( @sample_started_at, @sample_finished_at );
-- 2009 07 21 BC 3459a: Changed: SET @canarian_query_elapsed_msec = rroad_f_datediff_msec ( @canarian_query_started_at, @canarian_query_finished_at );       

PARAMETERS MONITOR_DEBUG_MESSAGES; 

-------------------------------------------------------------------------------------------------------------------------------------------------
MESSAGE STRING ( '****************************** CREATE PROCEDURE rroad_monitor_sample' ) TO CLIENT;

BEGIN
   DROP PROCEDURE rroad_monitor_sample;
   EXCEPTION WHEN OTHERS THEN
END;

-----------------------------------------------------------------------------------
CREATE PROCEDURE rroad_monitor_sample (
   IN  @sampling_id                        UNSIGNED INTEGER )

BEGIN

...

------------------------------------------------------------
-- Try the heartbeat query.

IF @ok = 'Y' 
AND COALESCE ( @proxy_owner, '' ) <> '' THEN 

   BEGIN -- handle connection problem

      SET @diagnostic_location = '204.a7';

      ------------------------------------------------------------
      -- Record the sample start time immediately before the first proxy call.

      SET @sample_started_at  = CURRENT TIMESTAMP;
      SET @sample_finished_at = @sample_started_at;

      ------------------------------------------------------------
      -- Perform the basic heartbeat or canarian query.
      -- This is done to calculate the heartbeat time as well check the connection.
      -- Note: The target database can be stopped and restarted, and the monitor should just keep trying to connect.
      -- Note: When Foxhound itself is is stopped and restarted, reconnection to target databases *should* be automatic.

      SET @sql = STRING ( 'SELECT dummy_col INTO @dummy_col FROM ', @proxy_owner, '.proxy_DUMMY' );

      SET @canarian_query_started_at  = CURRENT TIMESTAMP;
      SET @canarian_query_finished_at = @canarian_query_started_at;

      EXECUTE IMMEDIATE @sql;

      ROLLBACK; -- release locks, recover from possible failed proxy connection

...

END; -- rroad_monitor_sample

-- 2007 02 04 BC 1988a: Added: Column rroad_conn_property_name.sample _interval                      UNSIGNED INTEGER NOT NULL,                   -- 10 ultra, 10 fast, 60 slow
-- 2007 02 04 BC 1988a: Added: Column rroad_db_property_name.sample _interval                      UNSIGNED INTEGER NOT NULL,                   -- 2 ultra, 10 fast, 60 slow
-- 2007 02 04 BC 1988a: Added: Column rroad_eng_property_name.sample _interval                      UNSIGNED INTEGER NOT NULL,                   -- 2 ultra, 10 fast, 60 slow
-- 2007 02 04 BC 1988a: Added: Column rroad_sample_set.sample _interval                      UNSIGNED INTEGER NOT NULL DEFAULT 0,

...

-- 2009 07 11 BC 3430a: Added: Column temp_database_properties.MirrorMode             LONG VARCHAR,
-- 2009 07 11 BC 3430a: Added: Column temp_database_properties.MirrorState            LONG VARCHAR,
-- 2009 07 11 BC 3430a: Added: Column temp_database_properties.PartnerState           LONG VARCHAR,
-- 2009 07 13 BC 3432a: Added: Column temp_database_properties.ReadOnly               LONG VARCHAR,