Code Comments are not for tracking changes

Recently, I have been reviewing and correcting a lot of broken code, and in doing so, I have concluded that the number of developers who know how to properly use a bug tracker and version control is not nearly high enough (disclaimer – this is in a big faceless corporation).

Commenting Anti-Pattern

So what leads me to this bold conclusion? Have you ever seen code that looks like this:

begin
  -- start change for #3245
  v_myvar varchar2(100)
  -- end change for #3245

  for r in (select c1,
            /* start change for #9870 */
                        c2,
                        c3
            /* end change for #9870 */
              from foo_table
              where c1 in (1, 4, 6)
           -- commented out for #9870
           -- and c2  = 'val1'
           -- start change for #5678
               and c3 = 'val2'
           -- end change for #5678
             ) loop
    null;
  end loop;
end;

Ignore the fact that this code does nothing useful, and is PLSQL – its just there to illustrate a point.

The amount of code I have seen recently that has its changes tracked through inline comments that attempt to encapsulate every single line changed to fix a bug or add a feature is unbelievable.

Some may ask, what is the big deal? Well firstly, when I see code like this, it says to me, the developer who did this, doesn’t have any idea about version control – they are commenting every change to attempt to provide traceability and rollback ability to their change. Often, when I see this anti-pattern, I subsequently find the code hasn’t seen the inside of Subversion or anything like it.

Show me the code

If I ever dare to ask to see the changes that were made to fix a bug in code like this, I get handed a source code file and told to grep through it for the bug number – what scares me, is how do I know he remember to comment every single change? For a big change, it takes a lot of patience to do something like that! And then the Delivery Manager (DM) arrives at the developers desk and the conversation goes like this:

DM: Remember those 10 feature requests we agreed to deliver tomorrow, well how are they going?

Dev: No problems, we have them all finished and tested, ready to roll!

DM: Well, we have a problem – turns out the front end developers have not been able to complete feature #4567 and that impacts us – we need to deliver the other 9 changes, but not that one.

Dev: Huum, thats going to be a problem, #4567 was a big change – I don’t think we can get it out of there without a lot of effort!

DM: Don’t you have all this tracked in your version control system … back in my day we committed individual changes into SCCS and things like this were never a problem!

Dev: Gulp … guess its going to be a long night for me then sir …

Finally, don’t even get me started on how much harder the code is to read with all these annoying ‘starting/ending’ change comments.

A simple recipe

If you come across this rant, and have found yourself doing this, then stop a moment to think about how to use version control to track these changes for you – thats what is was designed for!

Here is a tip – when you have to fix a bug or add a feature to existing code:

1. Checkout your code
2. Make changes to whatever files are necessary and forget about pointless start/end change comments.
3. Get your test suite to pass your bug/feature (you have a test suite, right?)
4. Check in you code when its all done, quoting the bug number in the commit comment

Simple, clean code changes, traceable in a much more robust way and you can safely roll back the changes to any bug at any time.

The biggest mistake I see developers making with version control, aside from not using it at all, is to fix about 10 bugs, and then do a commit – please don’t do this, as it removes the ability to examime the changes made to fix a single bug, not to mention removing the changes if the need arises – which happens more than you may think, as our poor Dev found out above!

Advertisements