Tuesday, July 3, 2012

Thou shalt comment thine code


One the most difficult things to do as a developer or DBA is to attempt to understand someone else’s code no matter what programming language or database is being employed. As a former developer myself, I must admit outright that commenting my code used to be one of the last things on my mind when producing a functional module. Commenting code is like eating a healthy balanced diet, you know you should do it, but who needs the hassle? What I’ve discovered though is very often I would attempt to look at code I myself wrote long ago and the logic would be difficult even for me to follow. Most of the time I’d be asking myself “Why did I not comment this?”.  To this end I began employing a methodology for ensuring I included comments in my code, instead of viewing comments as a last step or a nice-to-have I’ve attempted to make it as part of the process I use when coding.
The steps for doing this are simple and can be applied in any environment:
1.       Type out as comments each major step of the script. Pay specific attention to the outcome of each step. 
2.       Flesh out the script with actual code
3.       Amend or Add comments to include particularly difficult to follow code blocks and sub-steps
4.       Include a header comment stating the objective of the script
The development of the code therefore should look like the example below. The example I’ve used is part of an allocation script to determine the destination of backups depending on the number of times copies have been completed to a drive:

Step 1: Type out in comments each major step of the script.
My script at this point would look like this:
-- Create the Control table
-- Insert inital Values
-- Determine which drive to send copies to
-- Check Number of times the drive has been used - each drive can only be used 2  times consecutively.
-- Set the next drive for use

Step 2: Flesh out the script with actual code
 USE [DBA]  
 GO  
 -- Create the Control table  
 CREATE TABLE [dbo].[BackupDrive]  
   (  
    [DriveID] [int] IDENTITY(1, 1)  
            NOT NULL,  
    [DriveLetter] [varchar](5) NULL,  
    [LastUsed] [int] NULL,  
    [Active] [int] NULL,  
    [NumOfUses] [int] NULL,  
    CONSTRAINT [PK_BackupDrive] PRIMARY KEY CLUSTERED ( [DriveID] ASC )  
     WITH ( PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF,  
         IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON,  
         ALLOW_PAGE_LOCKS = ON, FILLFACTOR = 90 ) ON [PRIMARY]  
   )  
 ON [PRIMARY]  
 GO  
 -- Insert inital Values  
 INSERT INTO dbo.BackupDrive  
     SELECT 'D:',  
             1,  
         1,  
         1  
     UNION ALL  
     SELECT 'E:',  
         0,  
         1,  
         1  
     UNION ALL  
     SELECT 'F:',  
         0,  
         1,  
         1  
Step 3: Amend or add comments to include particularly difficult to follow code blocks
Sometimes as a script develops the original skeleton may need to change or a sub-step(s) may need to be added that I did not foresee, step 3 ensures that these are explained and included in the comments. If you look at the full example below, there are more comments detailing the sub-steps of the allocation than I had originally planned in step 1.

Step 4: Include a header comment stating the objective of the script
/*Determine which Drive to copy backups to after each run of the daily backup job*/

 The full script will therefore look like this:

 USE [DBA]  
 GO  
 /*Determine which Drive to copy backups to after each run of the daily backup job*/  
 -- Create the Control table  
 CREATE TABLE [dbo].[BackupDrive]  
   (  
    [DriveID] [int] IDENTITY(1, 1)  
            NOT NULL,  
    [DriveLetter] [varchar](5) NULL,  
    [LastUsed] [int] NULL,  
    [Active] [int] NULL,  
    [NumOfUses] [int] NULL,  
    CONSTRAINT [PK_BackupDrive] PRIMARY KEY CLUSTERED ( [DriveID] ASC )  
     WITH ( PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF,  
         IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON,  
         ALLOW_PAGE_LOCKS = ON, FILLFACTOR = 90 ) ON [PRIMARY]  
   )  
 ON [PRIMARY]  
 GO  
 -- Insert inital Values  
 INSERT INTO dbo.BackupDrive  
     SELECT 'D:',  
             1,  
         1,  
         1  
     UNION ALL  
     SELECT 'E:',  
         0,  
         1,  
         1  
     UNION ALL  
     SELECT 'F:',  
         0,  
         1,  
         1  
 -- Determine which drive to send copies to   
 DECLARE @drive VARCHAR(10),  
   @Uses INT ;  
 SET @drive = ( SELECT  DriveLetter  
         FROM   dbo.BackupDrive  
         WHERE  LastUsed = 1  
             AND Active = 1  
        )  
   -- Update Number of times the drive has been used - each drive can only be used 2 times consecutively.  
 SET @Uses = ( SELECT  NumOfUses  
        FROM   dbo.BackupDrive  
        WHERE   DriveLetter = @drive  
       )  
 IF @Uses = 1   
   BEGIN  
     UPDATE dbo.BackupDrive  
     SET   NumOfUses = @Uses + 1  
     WHERE  DriveLetter = @Drive  
   END   
 ELSE   
   BEGIN  
    -- Set the next drive for use  
     DECLARE @MinDriveID AS INT,  
       @NextDriveID AS INT ;  
     SET @MinDriveID = ( SELECT DriveID  
               FROM  dbo.BackupDrive  
               WHERE  DriveLetter = @drive  
              )  
    -- If the last drive used is the absolute last drive that can be used, allocate to the first drive  
     IF @MinDriveID = ( SELECT  MAX(DriveID)  
               FROM   dbo.BackupDrive  
              )   
       BEGIN  
         SET @NextDriveID = ( SELECT MIN(DriveID)  
                    FROM  dbo.BackupDrive  
                    WHERE Active = 1  
                   )  
       END         
     ELSE   
       BEGIN  
         SET @NextDriveID = ( SELECT TOP 1  
                       DriveID  
                    FROM  dbo.BackupDrive  
                    WHERE DriveID > @MinDriveID  
                       AND Active = 1  
                   )   
       END       
    -- Update the affected records to current state  
     UPDATE dbo.BackupDrive  
     SET   LastUsed = 1, NumOfUses = 1  
     WHERE  DriveID = @NextDriveID  
     UPDATE dbo.BackupDrive  
     SET   LastUsed = 0, NumOfUses = 1  
     WHERE  DriveID = @minDriveID  
    -- Set the next drive as the drive to send to  
     SET @drive = ( SELECT  DriveLetter  
             FROM   dbo.BackupDrive  
             WHERE  DriveID = @NextDriveID  
            )  
   END  

Obviously I’ve included the CREATE TABLE and the Initial value insert in the above script for completeness, these statements will only need to be run once.
Commenting code this way ensures that the next time I read it, I’ll be able to easily determine what I was trying to achieve at each step, this will make changing my code easier in the future. As an added bonus, the comments above can become the foundation for proper documentation. I hope this was informative and helpful. That’s it from me for now.

Ronan Govender

6 comments:

  1. Always interesting when one of the most basic things in development requires justification...we are lazy by nature it seems. I was a compulsive commentor when I coded, and it was always interesting for me to read my own code the next day and find my comment and my code disagreed. Helped to find bugs before deployment though!

    Regardless of what the lazy techies try to convince us of, comments are essential. Thanks for this.

    ReplyDelete
  2. Yet again I personally feel the need NOT to comment on code. For the basic reason being that code should read as English and not at "gibberish". when our code is clean and readable one should never even need comments. This is only my opinion tho but do feel that when it comes to SQL especially, commenting might be the best thing since sliced bread.

    Nice post overall, love the diversity and different flavour! Thanks

    ReplyDelete
  3. Fanie, I agree with you that excessive commenting can sometimes add to the 'noise' level of code, making it more difficult to understand (I'm specifically talking about code, not SQL, where comments do help a lot). I've found that if I need comments to explain some code, it usually indicates that the code is too complex and needs re-factoring.

    However, I also try to at least comment most of my classes with a "mission statement" sentence starting with "This class is responsible for ...". This provides the reason for that class existing, and helps to ensure that your objects/classes focus on doing only one thing. This statement also helps you to determine whether a specific piece of functionality fits in with the purpose of that class.

    It also helps to do this on your methods, to again ensure that each method does not take up too much responsibility.

    ReplyDelete
  4. I like comments above a class giving a 2 line description of why it exists but....
    Comments are old school, needed because us old guys wrote spaghetti code that was covered in more mud code.
    Good code needs no comment, if you need to comment youve probably written it wrong.
    There also should be no business code in SQL, so no comments.

    ReplyDelete
  5. Agree with Mark here. If you need to comment, it means you need to right click and click on refactor :) Yes, keep SQL to (CRUD) simple and fast, thus no comments there as well.

    ReplyDelete
  6. My view is the only place one should comment is on Blog Posts

    ReplyDelete