info.michael-simons.eu

TL;DR: The older I get, the more diverse projects I have seen, the more often I refrain from giving someone an absolute advice. In almost any cases, there are many aspects to consider. In times when people hardly read documentation and often only headlines, an advice like “Stop Writing Code Comments” guarantees some clicks, but isn’t absolutely for the good nor appropriate in all scenarios.

At the beginning of this week, @java tweeted this:

Pointing to this blog post Stop Writing Code Comments. Brian seems to be a big apologist of the clean code movement. I can understand this to some extends, but moved away from “keep methods as small as possible” and similar quiet some extend. Sometimes it’s just necessary and as long as the logic in a bigger method stays testable, I don’t see an issue. And apart from that, what’s dirty code anyway?

What and how to comment

But I want to address some of the methods in the Medium post above. The name of the method is indeed good. But imagine the following:

/**
 * Finds all the employees with a given status. The method will return all employees with a given status, regardless
 * when they joined the company. The returned list will be immutable, changes to the employee objects will not be tracked.
 * 
 * @param status The employment status of the employees to find. Must not be null.
 * @return A list of employees with the given status or an empty list of no employee has the given status
 * @throws IllegalArgumentException when the status is literal null
 */
List<Employee> getEmployeesByStatus(Status status) {
  ...
}

As a user of that method I know now exactly what to pass to the method and what I can expect to return. I have an idea that the list may be empty, but is never null.

The employee class is badly named, that’s nothing to fix with a comment indeed. But chose a better naming and add some meaningful comment and things look bright:

/**
 * A person that is currently employed at the company. Instances of this class are not attached to the database
 * and represents a current employee purley from the business domain and can be savey passed around services.
 *
 * For other types of employees, see {@link RitiredEmployee} or {@link FiredEmployee}
 */
public class CurrentEmployee {
}

I do actually like this part:

public void sendPromotionEmailToUsers() {
  calculatePrices();
  compareCalculatedPricesWithSalesPromotions();
  checkIfCalculatedPricesAreValid();
  sendPromotionEmail();
}

There’s nothing much to add here. But let’s focus on sendPromotionEmail. It’s intentions are pretty clear, but I think it can be improved:

/**
 * Sends an email to all users eligible for a promotion. It does not include any retry mechanism, an e-mail is considered
 * sent when the configured SMTP server acknowledged the e-mail
 * An exception is thrown when the SMTP server is not reachable.
 *
 * When no users are eligible for promotion, no emails are sent
 * @see #getEmployeesByStatus
 * @throws RuntimeException when the SMTP Server could not be contacted
 */
void sendPromotionEmail() {
}

By coincidence I found this paper on The Use Of Sub-Routines In Programmes by David Wheeler. My friend Lukas quoted it in his talk 10 Reasons Why we Love Some APIS and Why we Hate Some Others. The paper – being from 1952 – summarises things neatly:

[…] sub-routines are very useful – although not absolutely necessary – and that the prime objectives to be born in mind when constructing them are simplicity of use, correctness of codes and accuracy of description. […]

Neither small and grokable methods nor comments alone help to create an understandable software system. One needs both.

On the obsolesce of comments

Brian speaks a lot about comments that becomes invalid and outdated. In my opinion, comments just needs to be treated as deliverable artefact as well and needs to be refactored with the code being documented. It’s easy as that.

Useless comments

I wholeheartedly agree with large chunks of commented code: Remove it, please! That’s what a version control system is for.

Regarding TODO comments: I use them occasionally to remind me of things I need to come back to in the future, which would block me from more important things at the very moment.
However, one alternative solution to that is to write failing tests: Those reminds you much better of things to fix.

Title photo by Marc Schäfer on Unsplash