Code review, or substantive criticism of yourself and others

Rafał Pieńkowski
Calendar icon
10 kwietnia 2020

You sit on a solution to a problem for several days. You come up with its solution. You design classes and methods. You're immensely proud of yourself... and suddenly you find out that you've named variables wrong, methods are unintuitive, the whole class to be rewritten, and in general you didn't fully understand the initial problem anyway. How to avoid a collision with the brutal reality, and how to objectively review the code of others? This article will give you a hint.

What exactly is code review and what is it for? Code review usually takes place after the implementation of a given functionality, at which time the other team members decide whether to implement the changes in the project or whether they still need some improvements. Comments and suggestions on code review can be divided into 3 levels:

Level 1 - TYPOs, typos and minor comments.

This is the most basic type of comments, the easiest for reviewers to spot.


As you can see from the example above, the addd method should be changed to add. Detail over detail, however, such comments are also needed to keep the code clean. Suggestions we know the names are also needed:


The method converts a collection of names - they should have at least 3 and (acct in this particular case) at most 30 letters, then converts all letters to lowercase. What can be changed here? Well, the name of the method, although explaining its implementation, is quite long, you can think of a better one:


The method name has been shortened, moreover, it does not reveal to the outside world the implementation details - the method "knows" what length the names should be - someone who calls the method does not need to know these details. The local variables describing the numbers 3 and 30 have also been extracted - they are no longer so-called "magic numbers". Notes of this style are also necessary as much as possible, but we should not limit ourselves to them only.

Level 2 - Structure of classes and methods, tests, design patterns

Adding this type of remark already requires a longer and more in-depth analysis. Let's consider again the example from the previous type of remarks:


Level 2 comments could be as follows:

  • Should the names collection be a field, or should it nevertheless be given as a method argument?
  • Should the names collection become a list, or would a set be a more appropriate form for it, however? Do we accept duplicates?
  • Should the applicableNamesLowercased method remain public? Perhaps we only call it inside a class, or in subclasses?
  • Have tests been written for the above method?

Going back to the initial example with the calculator:

  • Shouldn't the class be marked as final? After all, we are unlikely to extend the calculator class.
  • Shouldn't the add method be static? If the whole class is an util style class?
  • Or should the add method remain non-static after all, since static methods are more difficult to test?
  • Have tests been written for the above method?

As we can see, there are many more considerations that qualify for Level 2 - one can even say that in some nuances, as many people as many opinions. However, they should all have a common denominator - striving for the most readable code possible.

Level 3 - Business logic

To add a note of this kind, you need to know the project you are working in well, preferably holistically, not just a piece of functionality. Although sometimes common sense and a fair reading of the code is enough. Let's look at the method that has already appeared to us:


The minimum length has been changed! A programmer who does not fully understand the initial assumptions (minimum 3 letters) will write functionality that "works", even tested, and the tests pass... However, that programmer wrote the tests based on a false assumption! Level 3 style comments are highly valued by other programmers.

We have already learned about the different types of comments in code review. But what about the eternal wars (bars vs. spaces) or the different opinions of two programmers? Let's remember a very important word - CONVENTION. Do you think your colleague has named a class incorrectly? Pay attention first to whether he followed convention, that is, the rules adopted (by the team). Even "not-quite-clean code" in a large project, if kept to convention, is much more readable than "clean code" where each module has a different approach.

Let's also be open to substantive criticism from others - instead of "he's stuck again," think of the comments on code review as an opportunity for self-development and noticing other forms of vision. Every programmer feels satisfaction with his code, it's natural and healthy (it would be bad if there was no such satisfaction). However, let's not get too emotionally attached to one's solution. In the other direction, let's also not "take it out" on our teammates ourselves, exalting ourselves when we find some mistake in their code. Let's remember that code review has a common goal - to deliver quality code.

Read also

Calendar icon

16 maj

Implementation of the Omega-PSIR System at the Medical University of Varna

In May 2023, Sages had the honor of signing a contract for the first international implementation of the Omega-PSIR system with the M...

Calendar icon

28 marzec

RABATKA Action - Discover the spring promotion!

The annual spring promotion is underway: RABATKA Action, thanks to which you can save on open training from our proprietary offer.

Calendar icon

8 marzec

Don’t miss the opportunity - register for the IREB exploRE 2024 conference and meet the best experts in the field of requirements

We invite you to the IREB exploRE 2024 conference dedicated to requirements engineering and business analysis. The conference will ta...