Stop zealous with comments in code

Hello, Habr!

Today, a very controversial article is proposed for your attention, touching on an important aspect of the Clean Code philosophy . The author of the article takes the liberty of asserting that in most cases comments in the code are harmful, but also does not forget to indicate when to do without them.



Therefore - we read carefully and, in spite of everything, we comment.

Pure code should read like well-written prose - Robert Martin

As a rule, there is a pronounced correlation between bad code and code overloaded with comments. Comments are the most common feature of messy source code.

Every programmer should strive to write code that is so clean and expressive that comments are simply not required. The meaning of each variable, function, and class must be recognized by their name and structure. If you need to write a comment, this usually means that your code is not expressive enough. Whenever you write a comment, remorse must be cast on you.

When someone reads your code, it should be clear to him without comment what this code does. Properly named classes and functions should help the reader to follow the development of events, as if this is not a code, but a good novel. When a reader encounters a new function or class, their contents should not surprise him. Remember: the lion's share of the programmer’s working time is spent not on writing code, but on reading someone else’s code, which you have to understand.

Comments mask shoals

I often come across comments on variable or function names; such comments describe what the code does (or should do). Such comments clearly indicate that the programmer could not pick up a sufficiently expressive name, or that this function does more than one thing.

Proper naming of entities in code is an extremely important thing. By all means, try to name each piece of code so precisely that other developers understand it the first time and unambiguously.

// найти сотрудников по статусу
List find(Status status) {
  ...
}

In this example, the name findwas not sufficiently informative, so the author of the function had to compensate for this with a comment that describes what this function does. However, if the find function happens to be called from another module, it remains only to guess what it finds. What exactly is meant in this case by “find”? How does she find anything? Does it return what it finds? As Uncle Bob points out in his book Clean Code, if you need to write a comment, it means you were unable to properly express your thoughts in the code.

Who would like to delve into the commentary on each function to figure out what it does?

List getEmployeesByStatus(Status status) {
  ...
}

Now the purpose of this function is already obvious from its signature, and the comment becomes redundant. So, I come to the next aspect, explaining why the comments accompany the flaws.

Redundant comments

They only litter your code and are completely unnecessary. If you fill the code with a bunch of unnecessary comments, then the person reading it will learn to skip all comments to a single one, and because of this, you probably won’t read those that are really important.

// Эта функция отправляет электронное сообщение
void sendEmail() {
  ...
}
// В этом классе содержатся данные по сотруднику
public class Employee {
  ...
}
/**
 * @param title Это заголовок CD
 * @param author Это автор CD
 * @param tracks Количество треков на CD
 */
public void addCd(String title, String author, int tracks) {
  ...
}

The last example shows redundancy coming down from above. Many organizations require you to sign each function and class in this way. If your boss does the same, ask him not to.

Invalid abstraction level

If you have a long function, or you need to document the code, showing what part is being done, then you may be violating the following rules:

1. Each function should do exactly one thing.
2. Functions should be small.

Here is an example:

// Эта функция считает прайсы, сравнивает их с продажами, 
// промо-акциями, проаеряет, верны ли цены, а затем 
// отправляет пользователю письмо с промо-предложением 
public void doSomeThings() {
  // Вычислить прайсы
  ...
    ...
    ...
  // Сравнить вычисленные прайсы с промо-показателями по продажам
  ...
    ...
    ...
  // Проверить, верны ли вычисленные прайсы
  ...
    ...
    ...
  // Отправить пользователям промо-предложения
  ...
    ...
    ...
}

If you have fragments in your code that can be distinguished as independent functions, then refactoring is needed. If you were able to encapsulate each piece of logic in a separate function, then the code should be read exactly as a description of the actions embedded in it.

After refactoring, we have:

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

Instead of commenting on each piece of your code, make sure that any piece of logic is beautifully encapsulated in its own function.

First, readability is enhanced this way. The code does not have to be read in a row, line by line. It will be enough to read the name of the auxiliary function - and understand what it does. If we need to understand the details of a particular function in more detail, then you can always look into its implementation.

Secondly, this improves testability. In the above example, you can conduct a separate test for each function. If you do not encapsulate these individual functions, it will be difficult to test every part of the larger function sendPromotionEmailToUsers(). The more different things a function does, the more difficult it is to test it.

Finally, refactoring such code is also simplified. If we encapsulate each element of the logic in our own function, then in the future it will be easier to make changes to it, moreover, they will only affect the behavior of this function. If we have long functions with local variables that exist throughout the execution of the function, then refactoring such a function will provoke changes somewhere else due to the strong coherence of the code.

Commented out code

Commented code should be considered life threatening. Do not look into it, do not smell it, do not ask where it came from - just get rid of it. The longer it stays in the program, the longer the whole program will be with a shower.

/*
public void oldFunction() {
  noOneRemembersWhyIAmHere();
  tryToUnCommentMe();
  iWillProbablyCauseABuildFailure();
  haHaHa();
}
*/

If you try to uncomment such a code - who knows if it compiles at all? Will this piece of code disable some other piece of code? Just delete. If you need this code later, you can always get it in the version control system - you use the version control system, right?

TODO comments

Do not write TODO comments, but rather just ... do what you were going to? As a rule, such comments are simply forgotten, and later may become irrelevant or false. When another coder sees a TODO comment, how does he know if this operation has already been done? Delete such comments too.

The only case in which a TODO comment seems appropriate is when you expect a merge from a colleague. It is assumed that such a comment does not sag in the code for long, only until you can make corrections to the code and fix them.

If you feel that you need to write a comment, first try refactoring so that any comments in the code become redundant. - Martin Fowler

Comments lie

Another problem with comments is that they are false.

When Jimmy leaves a comment on a new feature that he just wrote himself, he thinks he is helping other developers who will later read his code. In fact, he sets a trap. His commentary may lie untouched for months and years until it turns into a heinous lie. Then, once, after hundreds of refactoring and changes in requirements, this comment will be disavowed by some distant module.

When you rewrite a line of code, how do you know if you are not canceling the comment in any other part of the code? You won’t know this. Therefore, comments must be uprooted .

public class User {
  ...
  // Здесь содержится имя и фамилия пользователя 
  String name;
  ...
}

Later, the developer decides to divide nameby firstNameand lastName.

// Обрабатываем информацию о сотруднике в зависимости от статуса
void processEmployees() {
  ...
  List employees = findEmployees(statusList);
  ...
}
// находим сотрудников по списку статусов
List findEmployees(List statusList) {
  ...
}

Bah! The comment is incorrect. It would be possible to update the comment so that it reflects the amendments, but do you really want to manually maintain all comments after any changes? You are a developer, not a documenter.

But such a comment is easy to notice and can be changed without problems.

Consider another example:

// Обрабатываем сотрудников в зависимости от статуса
void processEmployees() {
  ...
  List employees = findEmployees(statusList);
  ...
}
// здесь мы находим сотрудников по списку статусов
List findEmployees(List statusList) {
  ...
}

Later, someone can change the findEmployees function so that it finds employees by the list of names, and not by the list of statuses.

// Обрабатываем сотрудников в зависимости от их статуса
void processEmployees() {
  ...
  List employees = findEmployees(statusList);
  ...
}
// здесь мы находим сотрудников по списку статусов
List findEmployees(List nameList) {
  ...
}

First, the comment on findEmployees became irrelevant, and it needs to be changed. This is not a problem, right? Not like that .

The comment above has processEmployeesalso become irrelevant, and it also needs to be changed. How many more such comments have become invalid as a result of such a small refactoring?

Alternative solution:

void processEmployees() {
  ...
  List employees = findEmployeesByName(nameList);
  ...
}
List findEmployeesByName(List nameList) {
  ...
}

If you accurately and carefully name the functions, then no comments will be needed, and your code will not creep false.

When comments are normal

Among the developers, I know many fanatical supporters of detailed commenting on the code. Polemizing with them, I have to agree that sometimes comments are normal. Each time, entering a comment into the code, we do it reluctantly, but sometimes a comment is a necessary evil ...

Compound expressions

If you have a complex SQL statement or regular expression in your code, accompany them with a comment. Such commands are usually difficult to clearly and expressively express in code. Commenting on such an expression will seriously help fellow developers understand your code.

// формат по шаблону kk:mm:ss EEE, MMM dd, yyy
Pattern timePattern = Pattern.compile("\\d*:\\d*:\\d* \\w*, \\w*, \\d*, \\d*");

Warnings

If you need to warn other developers about possible side effects or troubles, then this code is appropriate to accompany a comment. Such comments can work like beacons warning of the mysterious behavior of the program - therefore, the code with them becomes much more valuable.

Explanation of Intent

If you can’t write clean code, admit it and write a comment. You are responsible for all the code you write, so never leave bad code without explanation.

If you need to write a comment, then make sure that it is in the right place. A comment located far from the code to which it refers will soon turn into a lie, therefore it must be destroyed. A comment describing a function or variable should be directly above it. If your IDE supports comment highlighting, make sure your comments stand out from the code.