Commenting your code

Watch out for extremes

Just like in politics, extremes are simplistic views and can be dangerous.

Extreme 1 - comments are useless

The most common pattern is developers that won’t write a single line of comment. Here are some stuffs they say:

• “The code speaks for itself, it’s simple.”
• “Use the Source, Luke!'.”
• “Look, I don’t have time to write comments.”
• “Comments are for beginners.”
• “When the code gets updated, comments get obsolete and confuse people”
• “Oh yeah, like I should write ‘this is a loop’, l-o-l dude”

They are right about one thing: there should be little comments in your code. Whenever possible, a comment should be replaced by explicit code.

Extreme 2 - comment everything

Because comments are usually a thousand times faster to read and understand, some developers will have the opposite position: “Comment every class, method and function”. The idea here is to make sure that every piece of code as an explanation in plain English (or another human language :) ). The obvious problem with this is that some code really doesn’t need any explanation - adding an unnecessary explanation will take up some space and might slow down other developers (because they read the comments). For example; I usually add a method to get a user name:

class User(...):
    ....
    def get_full_name(self):
         return u"{0} {1}".format(self.first_name, self.last_name)

There is no way commenting that method would help ! The method name is explicit, if you really need to know the details (is the title part of the full name), you can glance at the one liner implementation.

Useful comments

In my opinion, proper commenting is a great help. I love TDD and in a similar fashion I like the idea of writing a generic comment first (and review it once the commented piece of code is done). For those who argue that code should speak by itself; here is an example:

    class TwitterImport(object):
    
        def import(account):
            ...

Imagine that you got a server notification (Twitter API Permission denied) on a legacy project with the above class. The only way you’ll know for sure what this class does is by reading the code inside the import() method and by doing a search in the code and tests to see where it’s used from.

With that interface, you know that:

• this class probably imports Twitter feeds into the website database
  • you’re not 100% sure about this, it might be the other way around (example: creating feeds based on the News) !
• it uses an account
  • it’s probably a user account, but it could be a Twitter account or maybe the user organisation account, etc

What you don’t know:

• if there is only one Twitter feed
• if it’s called when doing something (as a sync task OR as an async task)
• if it’s called in a command / CRON job
• the developer intent:
  • this could have been a quick copy/paste and now that other social medias are coming to the platform, this should be refactoredwhat’s the account is (user account, user organisation account, Twitter account?)
• if the there is any requirements regarding that account

Basically, you know nothing Jon Snow !

Now let’s imagine that the original developer was kind enough to comment on that class:

class TwitterImport(object):
    """ Premium users can display their twitter feed on their page.
    That class helps managing all the Twitter feeds by caching them in the database.
    Could Do: see FacebookImport, should make it DRY when we add Google+
    """

    def import(account):
        """ Import last week tweets in the database for a user account. 
        The user must have a premium account. 
        This should be called everyday.
        """
        ...

This is much better but the comments on the import method are slightly verbose. Let’s replace some of them with actual code:

    def import_feed_into_database(account, last_days=7):
            """ Import tweets for a user account.
            This should be called everyday.
            """
            assert isinstance(account, myapp.Account)
            assert account.plan_is(PREMIUM_PLAN)

The import name is really generic. Replacing it with import_feed_into_database tell us:

• what we import
  • it’s the feed, not the twitter account details
• the direction
  • from Twitter to our website
• where we import it
  • we copy the feed in the database, not in a JSon file for example

The fact that last week tweets are imported is a valuable information but it’s easy to replace this with a parameter - making the comment unnecessary and giving more flexibility to the developer. last_days=7 just does that.

The comment “The user must have a premium account” is a fair comment but it does nothing. The assumption with that comment is that other developers always read comments and won’t forget. Instead of hoping for this; we can enforce it with a pre-condition: assert account.plan_is(PREMIUM_PLAN). This way, if a developer try to import a Twitter feed on a non premium user by mistake, he’ll receive a notification; then he would check why and realise that non-premium users don’t have a space to show their feed so that assert prevents to load the database with useless information.

Conclusion

Commenting is really important and should not be skipped. It’s however equally important to then look at your comments and ask yourself what could be replaced with actual code. Like many things in life, comments are better in small amounts.