Don't use comments, be expressive!

We need to talk! And it will be about comments.

Comments have nice properties. They are plain in English, and as such should be easy to understand. They are also able to summarize a whole block of code in a line of two, witch make it much faster to read. However, they have a huge defect, they can be out-of-date.

This article is going to explore other ways to have the same level of expression, the same level of concision, while being sure that what is written is always up-to-date.

If we want to replace comments by something else we need to identify their use-cases:

• High-level documentation. Explaining what a module does.
• Requirements of an interface. Listing all contracts of a function, class, …
• Example of use. To help a new user.
• Low-level documentation. Explaining how an algorithm works.

High-level documentation

/**
 * Atomic payment transaction. If the payment can't succeed, nothing is
 * modified and an exception is thrown with details about the failure.
 */
void transfer( /* ... */ );

Using plain English for high-level documentation is not necessary an issue, since the high-level architecture is mostly fixed. Therefore, it can easily be maintained.

High-level integration tests are a good complements to that kind of documentation, but don’t (and don’t have to) replace it. They should demonstrate how the product should be used with standard use-cases.

Requirements

void transfer(
    std::string amount,    /* 3 digits fixed-point precision */
    std::string currency,  /* 3-letters currency code        */
    std::string emitter,   /* 11 digits account number       */
    std::string receiver); /* 11 digits account number       */

Comment are not at all the best tool to express requirements. They can easily be forgotten during an update of the code, and quickly becoming out-of-date. Programmers may not read them, but what is even worse is that the computer totally dismiss them! It is much better to use your type system for this.

void transfer(
    Currency amount,
    Account emitter,
    Account receiver);

All the logic is going to be handled by small classes, instead of being spread at multiples places in the code.

struct Currency {
    int amount;
};
struct Euros: public Currency {};
struct USD: public Currency {};

struct Account {
    std::string owner;
};

Example of use

// in payment.h

/**
 * \Example
 * auto john_doe = Account{12345678901};
 * john_doe.setBalance(1000_euros);
 * 
 * auto jane_roe = Account{98765432109};
 *
 * // transfer 500 euros from John Doe to Jane Roe
 * transfer(500_euros, john_doe, jane_roe);
 */
void transfer(Amount amount, Account emitter, Account receiver);

The best way to show how a function or a class should be used is by creating a real example. To be sure that this example is always up-to-date, that example should be compiled and run regularly. As such, it should be a unit test. The other advantage of unit tests is that they act as a good source of documentation. If a system should have a given property, test it, and automate that testing with a unit test. They are complementary to language-level feature like correct use of your type system, contracts, annotations, const-correctness, … Together (test and language-level features) describes both the requirements of an interface, and describes how to use it.

// in payment.test.cpp

TEST_CASE( "Valid payment", "[payment]" ) {
   auto john_doe = Account{12345678901};
   john_doe.setBalance(1000_euros);

   auto jane_roe = Account{98765432109};

   REQUIRE_NOTHROW(
       transfer(500_euros, john_doe, jane_roe));
}

TEST_CASE( "Invalid payment", "[payment]" ) {
   auto john_doe = Account{12345678901};
   john_doe.setBalance(0_euros);

   auto jane_roe = Account{98765432109};

   // try to transfer 500 euros from John Doe to Jane Roe
   REQUIRE_THROWS_AS(
       transfer(500_euros, john_doe, jane_roe),
       BalanceTooLow);
}

Low-level documentation

If you see a piece of code like this one, what should you think?

int attack(
       std::string number_of_faces_on_dice,
       std::string raw_number_of_dices,
       std::string critical_threshold)
{
   // 1 - extract all values
   int number_of_faces = std::stoi(number_of_faces_on_dice);
   int number_of_dices = std::stoi(raw_number_of_dices);
   int bonus_damage = std::stoi(critical_threshold);

   // 2 - roll the dices
   int damage = 0;
   for (int i = 0; i < number_of_dices; i++) {
       damage += rand() % number_of_faces;
   }

   // 3 - add critical damage
   if (damage >= critical_threshold) {
       damage += 10;
   }

   return damage;
}

At the moment, this snippet is quite short, and therefore quite readable. However, there is a high chance that it will become quickly messy, if we start to add more and more requirements to our attack() function. The cause of the issue is the wrong level of abstraction used in the body of the function attack(). It’s easy to spot, because you have numbered comments that explains what is done. This can be totally replaced by creating a sub-functions to roll the dices, and change the interface to accept only proper types.

int attack(int number_of_faces, int number_of_dices, int critical_threshold) {
   int damage = roll_dices(number_of_faces, number_of_dices);
   add_critical_damages(damage, critical_threshold);

   return damage;
}

int roll_dices(int number_of_faces, int number_of_dices) {
   int sum = 0;
   for (int i = 0; i < number_of_dices; i++) {
       sum += roll_dice(number_of_faces);
   }
   return sum;
}

int roll_dice(int number_of_faces) {
   // TODO: use a better randomization function
   return rand() % number_of_faces;
}

void add_critical_damages(int& damage, int critical_threshold) {
   if (damage >= critical) {
       damage += 10;
   }
}

By using the right types for your input variable, and creating sub-functions, the maintenance of your code can be heavily simplified. It was also easier to spot that the randomization function used wasn’t the best that we could use.

Conclusion

You should not expect that someone who wasn’t able to clearly express his intent in code will magically be able to become clear when writing a comment. Try to refactor your code to make it as readable as plain English, and easy to follow. Instead of writing specification and example of use, use your type system, and write tests. This should significantly decrease your maintenance cost by simplifying your application.

The remaining comments will have much more value, because it will only be high level description of the whole system, as well as technical and architectures choices.