5 Best Practices for Commenting Your Code

One of the first things you learn to do incorrectly as a programmer is commenting your code. My experience with student and recently graduated programmers tells me that college is a really good place to learn really bad code commenting techniques. This is just one of those areas where in-theory and in-practice don’t align well.

There are two factors working against you learning good commenting technique in college.

  1.  Unlike the real world, you do a lot of small one-off projects as a solo developer.  There’s no one out there fantasizing about dropping a boulder on you for making them decipher your coding atrocity.
  2.  That commenting style you are emulating from your textbook is only a good practice when the comments are intended for a student learning to program. It is downright annoying to professional programmers.

These tips are primarily intended for upstart programmers who are transitioning into the real world of programming, and hopefully will prevent a few from looking quite so n00bish during their first code review. Code Review? Oh yeah, that’s something else they didn’t teach you in school, but that’s a whole other article, I’ll defer to Jason Cohen on that one.

So let’s get started…

(1) Comments are not subtitles

It’s easy to project your own worldview that code is a foreign language understood only by computers, and that you are doing the reader a service by explaining what each line does in some form of human language. Or perhaps you are doing it for the benefit of that non-programmer manager who will certainly want to read your code (Spoiler: He won’t).

Look, in the not too distant future, you will be able to read code almost as easily as your native language, and everyone else who will even glance at it almost certainly already can. By then you will realize how silly it is to write comments like these:

// Loop through all bananas in the bunch
foreach(banana b in bunch) {
    monkey.eat(b);  //make the monkey eat one banana
}

You may have been taught to program by first writing  pseudo-code comments then writing the real code into that wire-frame. This is a perfectly reasonable approach for a novice programmer. Just be sure to replace the comments with the code, and don’t leave them in there.

Computer: Enemy is matching velocity.
Gwen DeMarco: The enemy is matching velocity!
Sir Alexander Dane: We heard it the first time!
Gwen DeMarco: Gosh, I’m doing it. I’m repeating the darn computer!

-Galaxy Quest

Exceptions:

  • Code examples used to teach a concept or new programming language.
  • Programming languages that aren’t remotely human readable (Assembly, Perl)

(2) Comments are not an art project

This is a bad habit propagated by code samples in programing books and open source copyright notices that are desperate to make you pay attention to them.

/*
   _     _      _     _      _     _      _     _      _     _      _     _
  (c).-.(c)    (c).-.(c)    (c).-.(c)    (c).-.(c)    (c).-.(c)    (c).-.(c)
   / ._. \      / ._. \      / ._. \      / ._. \      / ._. \      / ._. \
 __\( Y )/__  __\( Y )/__  __\( Y )/__  __\( Y )/__  __\( Y )/__  __\( Y )/__
(_.-/'-'\-._)(_.-/'-'\-._)(_.-/'-'\-._)(_.-/'-'\-._)(_.-/'-'\-._)(_.-/'-'\-._)
   || M ||      || O ||      || N ||      || K ||      || E ||      || Y ||
 _.' `-' '._  _.' `-' '._  _.' `-' '._  _.' `-' '._  _.' `-' '._  _.' `-' '._
(.-./`-'\.-.)(.-./`-'\.-.)(.-./`-'\.-.)(.-./`-'\.-.)(.-./`-'\.-.)(.-./`-'\.-.)
 `-'     `-'  `-'     `-'  `-'     `-'  `-'     `-'  `-'     `-'  `-'     `-'

                 -It's Monkey Business Time! (Version 1.5)
*/

Why, that’s silly. You’d never do something so silly in your comments.

ORLY? Does this look familiar?

   +------------------------------------------------------------+
   | Module Name: classMonkey                                   |
   | Module Purpose: emulate a monkey                           |
   | Inputs: Bananas                                              |
   | Outputs: Grunts                                            |
   | Throws: Poop                                               |
   +------------------------------------------------------------+

Programmers love to go “touch up” their code to make it look good when their brain hurts and they want something easy to do for a while. It may be a waste of time, but at least they are wasting it during periods where they wouldn’t be productive anyway.

The trouble is that it creates a time-wasting maintenance tax imposed on anyone working with the code in the future just to keep the pretty little box intact when the text ruins the symmetry of it. Even programmers who hate these header blocks tend to take the time to maintain them because they like consistency and every other method in the project has one.

How much is it bugging you that the right border on that block is misaligned? Yeah. That’s the point.

(3) Header Blocks: Nuisance or Menace?

This one is going to be controversial, but I’m holding my ground. I don’t like blocks of header comments at the top of every file, method or class.

Not in a boat, not with a goat.
Why? Well let me tell you, George McFly…

They are enablers for badly named objects/methods – Of course, header blocks aren’t the cause for badly named identifiers, but they are an easy excuse to not  put in the work to come up with meaningful names, an often deceptively difficult task. It provides too much slack to just assume the consumer can just read the “inline documentation” to solve the mystery of what the DoTheMonkeyThing method is all about.

JohnFx’s Commandment:
The consumer of thy code should never have to see its source code to use it, not even the comments.

They never get updated: We all know that methods are supposed to remain short and sweet, but real life gets in the way and before you know it you have a 4K line class and the header block is scrolled off of the screen in the IDE 83% of the time. Out of sight, out of mind, never updated.

The bad news is that they are usually out of date. The good news is that people rarely read them so the opportunity for confusion is mitigated somewhat. Either way, why waste your time on something that is more likely to hurt than help?

JohnFx’s Maxim of Plagiarized Ideas :
Bad Documentation is worse than no documentation.

Exception: Some languages (Java/C#) have tools that can digest specially formatted header block comments into documentation or Intellisense/Autocomplete hints. Still, remember rule (2) and stick to the minimum required by the tool and draw the line at creating any form of ASCII art.

(4) Comments are not source control

This issue is so common that I have to assume that programmers (a) don’t know how to use source control; or  (b) don’t trust it.

Archetype 1: “The Historian”

     // method name: pityTheFoo (included for the sake of redundancy)
     // created: Feb 18, 2009 11:33PM
     // Author: Bob
     // Revisions: Sue (2/19/2009) - Lengthened monkey's arms
     //            Bob (2/20/2009) - Solved drooling issue

     void pityTheFoo() {
          ...
     }

The programmers involved in the evolution of this method probably checked this code into a source control system designed to track the change history of every file, but decided to clutter up the code anyway. These same programmers more than likely always leave the Check-In Comments box empty on their commits.

I think I hate this type of comment worst of all, because it imposes a duty on other programmers to keep up the tradition of duplicating effort and wasting time maintaining this chaff. I almost always delete this mess from any code I touch without an ounce of guilt. I encourage you to do the same.

Archetype 2: “The Code Hoarder”


     void monkeyShines() {
          if (monkeysOnBed(Jumping).count > max) {
             monkeysOnBed.breakHead();

             // code removed, smoothie shop closed.
             // leaving it in case a new one opens.
             // monkeysOnBed.Drink(BananaSmoothie);
          }
     }

Another feature of any tool that has any right to call itself a SCM is the ability to recover old versions of code, including the parts you removed. If you want to be triple super extra sure, create a branch to help you with your trust issues.

(5) Comments are a code smell

Comments are little signposts in your code explaining it to future archaeologists that desperately need to understand how 21st century man sorted lists of purchase orders.

Unfortunately, as Donald Norman explained so brilliantly in The Design of Everyday Things, things generally need signs because their affordances have failed. In plain English, when you add a comment you are admitting that you have written code that doesn’t communicate its purpose well.

Sign:”This is a mop sink.” Why would that be necces… oh.

Despite what your prof told you in college, a high comment to code ratio is not a good thing.  I’m not saying to avoid them completely, but if you have a 1-1 or even a 5-1 ratio of LOC to comments, you are probably overdoing it. The need for excessive comments is a good indicator that your code needs refactoring.

Whenever you think, “This code needs a comment” follow that thought with, “How could I modify the code so its purpose is obvious?”
Talk with your code, not your comments.

Technique 1: Use meaningful identifiers and constants (even if they are single use)

     // Before
     // Calculate monkey's arm length
     // using its height and the magic monkey arm ratio
     double length = h * 1.845; //magic numbers are EVIL!

    // After - No comment required
    double armLength = height * MONKEY_ARM_HEIGHT_RATIO;

Technique 2: Use strongly typed input and output parameters

      // Before
      // input parameter monkeysToFeed:
      // DataSet with one table containing two columns
      //     MonkeyID (int) the monkey to feed
      //     MonkeyDiet (string) that monkey's diet
      void feedMonkeys(DataSet monkeysToFeed) {
      }

     //  After: No comment required
     void feedMonkeys(Monkeys monkeysToFeed) {
     }

Technique 3: Extract commented blocks into another method

      // Before

      // Begin: flip the "hairy" bit on monkeys
      foreach(monkey m in theseMonkeys) {
          // 5-6 steps to flip bit.
      }
      // End: flip the "hairy" bit on monkeys

     // After No comment required
     flipHairyBit(theseMonkeys);

As an added bonus, technique 3 will tend to reduce the size of your methods and minimizing the nesting depth (see also “Flattening Arrow Code”) all of which contribute to eliminating the need for commenting the closing tags of blocks like this:

            } // ... if see evil
         } // ... while monkey do.
      } // ... if monkey see.
    } // ... class monkey
  } // ... namespace primate

Acknowledgments

Several of the ideas presented here, and a good deal of the fundamental things I know about programming as part of a team, I learned from the book Code Complete by Steve McConnell. If you are a working programmer and have not read this book yet, stop what you are doing and read it before you write another line of code.

Advertisements

Your resume. It’s the little things that hurt. (The Programmer’s Guide to Getting Hired)

A skill that emerges naturally for managers after conducting a relatively small number of recruiting efforts is the ability to recognize common anti-patterns in resumes that are contra-indicators of good developers. I’m not talking about the major faux pas that have been repeatedly covered in generic articles about job hunting, but rather the nuanced messages that are communicated to a technical hiring manager who has learned to  read between the lines. This article is intended as cautionary advice to help prospective developers avoid the most common and damaging mistakes beyond simple typos and grammar considerations.

Risk Level: For senior candidates, most of the mistakes described below are fatal to your candidacy. For more entry level technical positions I am more likely to let these types of issues slide, but do use them to differentiate candidates that are otherwise equally qualified, so it still makes sense to be mindful of them.

 

Tip #1: Don’t be hyper-specific

One of the most effective ways to portray yourself as a novice or one-trick-pony is to pile on the trivial details when describing past jobs or projects. I understand that you are proud of your accomplishments, and know how easy it is to get wordy when drafting the Book of Moi, but remember that a resume is a brochure, not a biography. The prime objective of a resume is to get you booked for an interview, not to close the sale. The interview is a much better forum to brag about the implementation specifics of the “Monolith 3.0 Infrastructure Upgrade Project” because it allows you to adjust the message as you deliver it to hone in on whatever aspects resonate with the interviewer.

The key problem with being too specific arises from the undesirable subtext it creates. I think this is demonstrated well in the following excerpt from a real resume submitted for a Web developer job that I was trying to fill.

“Created stored procedures to fetch query results from the database for higher performance. Involved in writing complex SQL queries that required data from multiple tables using inner and outer joins.”

Gems of this type are hardly rare. I see variants of this exact statement so often in developer resumes that I can now spot them as quickly as if they were marked with a yellow highlighter pen. My mind automatically drifts to the same questions every time I see something like this…

Question 1:  Why is this person making such a big deal about menial tasks like writing a stored procedure and using specific join types?
Option A: They find writing SQL/Stored Procs very difficult and consider it a major accomplishment to have done so.
Option B: They didn’t have much to talk about in the resume and are padding it with trivialities to seem more qualified.
Option C: They just learned about joins and are proud of that accomplishment.

Question 2:  Why call out that they wrote the procedure specifically to query data?
Option A: Maybe they have never written a query that modifies data, are they that inexperienced?
Option B: Are they trying to educate me about what a stored procedure is? Do they think this isn’t common knowledge? Perhaps it isn’t to them.

Another example from the same resume:

“Designed online store by using ASP.Net. This system allowed user to order the products from the store and user can accept the order or cancel the order. “

Question 3: Did this person do substantial work on the project or just write the code for the “Accept” and “Cancel” buttons?

My advice to the author of this resume would be to take Vince Lombardi’s advice and “Act like you’ve been there before.” By assuming the details, it conveys a tone that the candidate considers the specifics of optimizing queries as routine and not worth bragging about. The discussion about the specifics of the clever optimization technique are best saved for the interview. Here is a reworked version that generalizes the same sentiment without dwelling on the trivia.

“Substantially improved performance of company’s core accounting package by profiling and optimizing database access code.”

Another major problem with getting too far into the weeds on a resume is that it implies that the resume is a comprehensive. By being asymmetrically specific about experience with various technologies, it has the effect of implying that the applicant is less competent in skills that are described more generally and not skilled in those that are not explicitly mentioned. In the above example, by specifically calling out experience with SELECT queries, it leads the reader to the conclude that the candidate might not be able to form other types of queries.Be mindful that what you do say often speaks volumes about what you don’t.

Tip #2: Don’t egregiously add keywords in the narrative sections

It could be argued that being specific in a resume gives you an opportunity to get all the buzzwords in, and this is true. The gatekeepers to most jobs are recruiters and HR people who use keywords from the job requisition to locate candiates on sites like monster.com and internal recruiting databases often without understanding their meaning. For this reason, I like the idea of separating out the keywords in to a separate “skills” section on the resume. This makes it easy for recruiters to scan your list of competencies by grouping them together, and makes the narrative parts easier to read for the hiring manager who doesn’t need to be prejudiced by that level of detail.

Here are a couple of examples from resumes that work a little too hard to jam keywords into the job history.

“Debugged and fixed the bugs which reported in TestTrack Pro by Seapine Software, Inc. according to SDLC.”

“Designed the database in SQL Server using database normal forms to prevent data redundancy and ER (Entity Relationship) diagrams.”

Although it is amusing, I’ll forgo a discussion on whether using normal forms actually “prevent ER diagrams.” If only…