Code Review good practices: guide for beginners
How To Make A Good Code Review? Where to start? What to remember? What mistakes should you avoid? What CR tools to use? This article will answer all of these questions, and maybe even make you fully understand that effective code inspection can work wonders not only for the product itself, but also for the development team.
Code Review in (short) theory
Code Review is a practice that is used nowadays in almost every software development company. It allows you to detect errors at an earlier stage of the CI / CD cycle and reduce the risk of their later occurrence. It is also a great opportunity to exchange knowledge between programmers - both knowledge about good coding practices and the domain knowledge of a given product.
By definition, CR is most often explained as follows:
Code Review (also: CR, code inspection, code review or code reading) - is a practice involving the review of code written by a programmer by another person (most often a teammate) in order to reduce the risk of errors and adapt it to the adopted code company standards and assumptions related to architecture.
More cleverly: "Before anything goes into production, show your code to someone on the team and he will check it out if it's all git."
And that's about as much theory as we can find on the Internet.
Code Review in real
In a separate article you can review specific examples of a well and badly done Code Review in code.
Anyway, in fact, there are different forms of code review. Some people do code review very carefully, "clinging to" every variable or method name and "get outraged" when they see unnecessary comment. In such software houses, disputes often arise between programmers trying to convince others to be right. Ultimately, it ruins the team's morale and the overall atmosphere. Others treat code review as an unpleasant duty and the need to tick the next checkbox to release the next version of the product as soon as possible. Here, too, the code review isn't working as it should.
Finally, we find companies that follow code review practices to get the most out of them:
- They use it to implement new developers into existing code.
- They share knowledge, enabling less experienced programmers to reach the next level.
- By guarding each other against using bad solutions, they take care not to incur technological debt.
- Ultimately, they also improve relationships among team members, creating a friendly and developmental work environment.
The differences between these different approaches raise essentially two questions, which we will answer later in this article:
- How to make our code review a positive event for both us and the other developer?
- How To Conduct An Effective And Effective code review?
Code Review Good Practices
Rule # 1: Less is more
Browsing too much code at a time is extremely tiring. When we focus on a single fragment for a long time, our efficiency drops and the risk of missing a bug increases. Therefore, it is definitely better to review small code snippets. That way, doing CR doesn't tire that much.
Many studies have already been carried out, with one conclusion drawn:
Small commit -> Short review -> Better code
This should not be misunderstood! A short code review does not mean that we should race against colleagues who can review as many lines of code as possible faster!
To find a good benchmark, we can use the averages compiled by Google from their Modern Code Review case study as a good result. As for the number of files viewed:
- 35% of code review concerned changes to only 1 file,
- 90% of the changes did not fall within 10 different files.
Of course, the number of files is not quite a good measure if you are working in an environment where one file may have several thousand lines of code. For people dealing with such a long code, it is better to take the average number of lines of code oscillating around 24 (data from the same Google study).
Averages are great examples of good practice, but you should also be aware that even Google has departments where the median line of code per CR is 250. It just happens.
Rule # 2: Code Review Checklist
In order to ensure that the code review process is always complete and that nothing essential escapes our attention, it is extremely useful to make a list of the most important things to check in your code. It will be best if it takes the form of checkboxes.
Quite nice examples of CR checklists can be easily found on the Internet (eg HERE ). However, it is worth treating them only as the first version of our own list, which we will modify and adapt to our current situation over time.
If our code is very messy at the start, we don't have any tests in it, and what's more, without much work we are not able to introduce such tests, there is no point in creating a 50-point checklist, where we will take into account all code review good practices used by others. In such a situation, when each time half of the checklist points cannot be met and we can only put minuses in their place, such a checklist will have a demotivating effect and discourage us from further work at the beginning of our adventure.
At the beginning, a much better solution will be a short list that will increase the quality of our work, but will not paralyze us with the amount of negative feedback.
An example of a short code review checklist for beginners:
- Do I understand the code I am reading?
1A. Are the variable names correctly selected?
1B. Are the methods well named and parameterized?
2. Does the code comply with generally accepted encoding standards?
3. Does the code fulfill what is described in the task?
The above simple code review checklist will allow us to quickly and easily start working on CR and improving the code. After just a few iterations, we will be able to see that our code is starting to look a little bit better - according to the scout principle, we clean up a bit. Once we get into practice and get into the habit of avoiding these basic mistakes, we can begin to add more points to this list.
Examples of further points to be added to our code review checklist in subsequent iterations:
- Does the code meet the SOLID rules?
- Are there any unnecessary duplications in it?
- Do we manage exceptions well?
- Have the appropriate tests been written?
As we have already mentioned, these are just sample points. The structure of such a checklist should be the result of joint work of the team and, importantly, it should be a list that is systematically updated as our code and our own knowledge develop and improve.
Rule # 3: Code Review Tools
It is said (not without a reason) that a good developer is a lazy developer, and this is because when he does not want to do something himself, he will always find a simple solution that will replace him at work or at least help him in it. This principle also applies to code review, and while we are not yet able to completely exclude humans from this process, there are already couple tools we can use to make our CR easier.
The purpose of this article is not to cover all the code review tools available, but the following three (triggers, static code analysis, and code standards) are really worth knowing:
Triggers
In order to automate the processes related to code review by using ready-made tools, we must be able to start them automatically. In this topic, solutions such as git hooks and bitbucket pipelines, i.e. tools built into our code repositories, can help us.
If for any reason we cannot or do not want to use them, solutions such as Jenkins, Drone.io or Travis are suitable for us.
Static code analysis
In order to facilitate the monitoring of the code, we may subject it to a process of static analysis.
Thanks to this process, we will receive various types of information, such as cyclomatic complexity, the total number of lines of code in the project, or the average number of lines of code in the file. This data will allow us to follow the development of our system and react when we observe a negative trend.
For example, if the average number of lines of code in release-to-release files continues to grow, it may be a sign that we are not working properly on breaking the code down into small classes. After all, small classes are easier to maintain and to test them later.
Code standards
If in our software house the entire team works (or tries to work) according to a certain set coding standard, then there is also a tool that will automatically make sure that nothing that does not meet our standard ends up in our production version. Such a tool for the PHP language is, for example, PHP CS Fixer, which not only allows you to validate the standards, but also can adapt the code to a defined standard.
Rule # 4: Make it Matter
The last point, but probably the most important of all — make it matter! Regardless of the tools chosen, standards adopted and the amount of time spent, the code review process and its goals must be well understood throughout the development team. Responsibility for this rests with everyone - both those who check the code and those who write it.
The duty of the person checking the code is not only to uncheck the checkboxes on the list we have prepared. Such a person should also direct his comments and reservations to the code in such a way that its author clearly understands the impact of a given change or its lack and that he could learn something from such feedback. Only criticism properly communicated and constructive has a positive effect on the development of another human being. Taking care of this aspect will also improve the overall atmosphere of work in the team. After all, who wouldn't want to work in a place where they can constantly develop their knowledge and skills?
In turn, the author of the code must learn to distance himself from his own work - he cannot take all comments personally. It is extremely important that the comments we receive as a result of code review are treated as an opportunity to learn and see other possible solutions. "We all want the product to be of the highest quality" should be an absolute pillar of each code inspection.
An important element is also a good embedding of CR in the software development process adopted in a given company. We cannot allow a situation where someone devotes a large amount of time to code review, drawing conclusions and observations that could improve the code, and then this knowledge ends up in a place where no one returns to it. These situations cause frustration for those who apply code review. In a well-organized process, there is always time to improve the code.
Finally, an important thing: Disputes, conflicts and dissenting opinions
An important element of the code review process is also the development of a dispute resolution mechanism between programmers who cannot communicate themselves. Neglecting this element can ultimately lead to the emergence of chronic conflicts and an overall deterioration of the atmosphere. It can also make us abandon the code inspection process altogether, for all its advantages and benefits.
In a situation where the person reviewing the code raises a comment that the author does not agree with, each team should have a simple and quick mechanism for making the final decision. This could be a quick question for another developer / manager for their opinion or arranging a short meeting to make a joint decision.
Author of the article:
Mateusz Antkowiak | PHP developer Transparent Data
Professional programmer for almost a decade. He began his journeys with code from such exotic destinations as Lazarus Pascal or Delphi, and now he is drawn into the stable waters of object-oriented programming. He has extensive experience in process automation, which is useful in his current RegTech tools in Transparent Data.
You might also like:
