Comment Battles

Hey there!  Hi.  I’ve been a programming professionally for 18 years now.  I’ve made some observations during that time that I think are sort of interesting.

I haven’t posted anything here in over 3 years.  I’ve been keeping a list of topics on the side and figure it’s time to start logging some of them.  Today’s topic is: avoid comment battles in code.

My first job out of college was at Activision.  I worked on a UI Library for displaying dialogs, buttons, text, etc, in-game.  Back then it was common to write your own blitting code.  BitBlt()!  Well, I wrote some form of a blitter for some reason and I was deciding whether to replace an internal call to memmove() with some assembly code or other home-brew solution.  I did some investigation and found out that memmove() was already very fast.  It did stuff like copying the initial bytes until it got to DWORD boundaries, etc, then did more massive copies of bigger chunks of data at a time.  That’s what I wanted to do with my custom version of memmove().  Realizing it was already done, I instead decided to leave the stock call to memmove() in place and put a comment over it:

// Don't even try to make something faster than memmove().

A few weeks later, while digging through that code again, I found that someone else on the team wrote some assembly and replaced my simple call to memmove() with their home-brew “faster” solution.  But he left my comment in place, followed up by one of his own:

// Yes, try.  It's not that hard.

So, looking at the work of this superior programmer on my team, I found that he did the memory alignment improvement I was thinking of doing originally.  The exact one that memmove() already does.  So, my next step, of course, is to revert his work, then add the comment:

// Congratulations!  You just implemented a duplicate of memmove()!

But no… that is not productive.  At this point, you have to ask yourself if it’s worth a follow-up to this now established comment battle in code, or just talking with the original author to work it out.  That’d be the mature thing to do.  I actually did neither.  I just left the custom memmove() implementation in place and went on with my current tasks.

My title back then was Junior Programmer.  I even got a box of business cards with that title printed on them under my name.  So prestigious!  ”I’m not a programmer.  I’m much worse than that.  I’m a Junior Programmer!  Get it right.”  These days we’re not even called programmers.  No, we’re… engineers!

Now that I’m a respectable… software engineer, I’m much more professional about these things:

  1. Don’t just reply to a comment in code followed up by changing everything around.  Instead, strike up a conversation with the original author and work things out first.  Maybe I was wrong, and it’s possible to write something faster than memmove().  But in case I was right, having a discussion and possibly doing a bit more research might save implementation time.  Not to mention the reliability of using something from the language’s standard library.
  2. Keep your comments informative.  My original “// Don’t even try” comment was not useful. Instead, something like “// memmove() does DWORD-aligned copies and so is optimal for our needs” would have been better.  Or, no comment at all!  Use memmove() because it does what you want and is self-explanatory.  Then profile your code at run-time.  Is the memmove() a bottleneck?  If not, leave it alone and go optimize real hotspots.

Bottom line: communication is important.  Have arguments and discussions outside of code.  Come to a consensus.  Then implement that.