Friday, February 3, 2012

Coding Good Comments: Modification History

The easiest kind of comment to write is Modification History: A comment which identifies the "What?" and "When?" of each change made to the program code and serves to answer questions like "Did I remember to make some change?"

Too bad Modification History comments are also be the least important kind of comment, certainly less important than Exhortations and Explanations, probably less important than Section Titles.

Nevertheless, Modification History comments can be helpful when they provide a concise overview of changes made from the developer's point of view... with the emphasis on "concise" and "developer's point of view" since version control utilities and even source code comparison programs can provide all the excruciating syntactical detail you would ever need.

Example 1

The first few lines of Foxhound's most important and most heavily-modified stored procedure looks like this:
-- 2007 02 04 BC 1988a: Date begun.
-- (see bottom of file for earlier modification history)
-- 2011 12 05 BC 4025a: Published: Foxhound GA 1.2.4025a
-- 2011 12 25 BC 4033a: Removed: Old commented-out code.
-- 2012 02 19 BC 4046a: Added:   SELECT ... rroad_global_options.enable_schedules   INTO ... @enable_schedules   FROM rroad_global_options;
-- 2012 02 19 BC 4046a: Changed: Comment renumbered: -- Step 5 - Check to see if sampling has been turned off. 
-- 2012 02 19 BC 4046a: Changed: Comment renumbered: -- Step 6 - Get the properties. 
-- 2012 02 19 BC 4046a: Added:   Code section: -- Check the connection count.
-- 2012 02 19 BC 4046a: Added:   Code section: -- Check the connection sample schedule.
-- 2012 02 19 BC 4046a: Changed: IF @connection_properties_should_be_retrieved = 'Y' THEN

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

Each Modification History comment is limited to one line, and includes the date, developer's initials and the build number which first included the change.

Some changes are eyecatchers, like "Published: Foxhound GA 1.2.4025a" which server to identify big events in the lifespan of the application.

Other changes include snippets of code to facilitate future searches: "Added: SELECT ... rroad_global_options.enable_schedules INTO ... @enable_schedules FROM rroad_global_options;"

When whole sections of code are affected, the Section Title comment is included, as in "Added: Code section: -- Check the connection count."

Generally speaking, only the most recent Modification History entries stand a chance of being important to the developer, which is why the older entries are moved out of the way: "(see bottom of file for earlier modification history)". In the case of this module there are 387 lines of Modification History comments at the bottom of the module... someday, they might be deleted, but they're not in the way and from time to time they're useful, so the effort required to choose the cutoff-point for each module has never seemed worthwhile.

Example 2

It is tempting to code Modification History comments down in the body of the code, close to the affected lines, especially when the effort required to make the changes was very high... but

Resist The Temptation!

Modification History comments just . . . aren't . . . that . . . important! ...and it's just that kind of clutter that gives comments a bad name.

With every rule, however, comes an exception, and Foxhound has exactly one. In the previous article on Exhortations this comment was described:

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.                *****
-- ***********************************************************************


The "MUST ALSO BE DOCUMENTED" exhortation is put into effect with a series of Modification History comments copied from the first module and embedded down in the code of the second:
-- 4026-9999 eighth historical era

   -- Changes requiring work in this module...
   -- 2011 12 18 BC 4026a: Added:   CREATE TABLE rroad_schedule (
   -- 2011 12 18 BC 4026a: Added:   CREATE TABLE rroad_schedule_day_entry (
   -- 2011 12 18 BC 4026a: Added:   CREATE TABLE rroad_schedule_period_entry (
   -- 2012 02 20 BC 4047a: Added:   Column rroad_alert.email_status                LONG VARCHAR NOT NULL );
   -- 2012 02 20 BC 4047a: Added:   Column rroad_all_clear.email_status                LONG VARCHAR NOT NULL );
   -- 2012 02 20 BC 4047a: Added:   Column rroad_alert_cancelled.email_status                LONG VARCHAR NOT NULL );

   -- Changes not requiring work in this module...
   -- 2011 12 15 BC 4026a: Changed: Comment: rroad_alerts_criteria.display_alerts marked "-- no longer used"
   -- 2011 12 27 BC 4033a: Added:   Column rroad_global_options.enable_schedules           VARCHAR ( 1 ) NOT NULL DEFAULT 'Y' CHECK ( @enable_schedules IN ( 'Y', 'N' ) ),
   -- 2011 12 27 BC 4034a: Added:   Column rroad_schedule.enforce_this_schedule            VARCHAR ( 1 ) NOT NULL DEFAULT 'Y' CHECK ( @enforce_this_schedule IN ( 'Y', 'N' ) ),
   -- 2011 12 27 BC 4036a: Added:   Column rroad_sampling_options.activate_sample_schedule VARCHAR ( 1 ) NOT NULL DEFAULT 'Y' CHECK ( @activate_sample_schedule IN ( 'Y', 'N' ) ),
   -- 2011 12 31 BC 4041a: Changed: rroad_schedule_day_entry.day_number ... CHECK ( @day_number BETWEEN 1 AND 7 ),
   -- 2011 12 31 BC 4041a: Changed: rroad_schedule_period_entry.period_number ... CHECK ( @period_number BETWEEN 1 AND 672 ),
   -- 2011 12 31 BC 4041a: Changed: rroad_schedule_period_entry.period_code ... CHECK ( @period_code IN ( 'Y', '.' ) ),
   -- 2012 02 17 BC 4044a: Added:   Column rroad_sampling_options.schedule_request         VARCHAR ( 100 ) NOT NULL DEFAULT '',

Ugly, but easy to do, and easy to check later on to see if anything got missed... and yes, "checking later" is a frequent task.

The Bottom Line

If you're not going to do a thorough job of writing Modification History comments, then don't bother trying... nothing is more wasteful than a poorly executed task that results in a work product nobody uses.

"Thorough" doesn't mean "provide the same level of detail as a source code comparison program."

It doesn't even mean "document every trivial syntax error" that you fixed... but it does mean "list every non-trivial maintenance task you performed on the module"... on the code and on the other comments.

What? All of a sudden comments aren't important? Well, if that's how you feel, see "don't bother trying"... most don't.


No comments: