Code Review: Ideas for Improvement!
Code review is one of the pillars to ensure the quality of the developed product. This process is truly delicate and can sometimes lead to significant conflicts among developers, which may even extend beyond the professional realm. The technical level of a developer can be assessed through the review of their code, but it is essential to prepare the ground properly to conduct a “balanced” evaluation. In this article, I will try to present my vision of code review through examples drawn from my personal professional experience.
What is code review?
Code review is a software quality process in which code written by one or more developers can be “manually” analyzed by the rest of the team. During the review, developers will try to ensure that the code achieves the objective of the created ticket and also adheres to the coding standards established within the team. Reviewing a new and substantial feature often takes more time for verification than fixing a simple bug. The code review process always depends on the culture and rigor of the team. I have worked with developers who review code per commit (requiring proficiency in Git in such teams) and others who quickly analyze the code vertically without delving into the details of the ticket. For me, code review is sacred! (It’s not just the job of the lead developer or tech lead). Not to bother people but mainly to have visibility into all the code that exists and to understand what we want to deliver to our clients. Imagine that the developer who created the functionality is not here with us. No worries, the people who have done the review or pair programming are here to tackle a subject related to its development.
How I review code
I assume that the team has established clear code styling. Suppose I have to review code for a new colleague, whether they are senior, experienced, or junior. My approach is always to pair program with them, calling them and discussing directly in voice helps me better understand their code and ensure that we have developed all possible use cases and adhered to the acceptance criteria defined in the JIRA ticket, for example. When it comes to direct exchange, experience has taught me that it is always a matter of modesty and how the person sees themselves. It is easier to pair program with someone humble and intelligent who does not see themselves as a programming guru or Turing of development. Indeed, reviewing code with these developers allows you to improve your skills, appreciate their way of thinking, and draw inspiration from them. Unfortunately, they are not many. They are rare gems!
I generally break down my review into two main parts:
The first part is a vertical review of the code where I try to see if the code adheres to the code styling. However, we use the Swift Lint tool daily. Indeed, it helps us tremendously to have code that adheres to our code styling. Also, a solution like SonarQube helps save time and perform static code review before launching the pull or merge request.
The second step is to delve into the JIRA ticket details (Here comes the serious part). I try to read the business requirement very carefully (I already have an idea about all the tickets in the sprint, so I’m not discovering them, but just making sure that nothing has changed in the meantime! You never know :)). I especially verify that the proposed solution to handle the nominal case fits well with the architecture and how I would do it myself if I were coding this feature!
This is where things can unfortunately go wrong. We agree that architecture ensures a good part of the code quality and allows developers not to diverge and to create a kind of pattern of how we think when we code, but that’s not enough. We need to create what I call a thinking model within the team while keeping imagination and creativity. This pattern or ergonomic way of thinking is created over time within the team, and everyone will feel that there is a DNA or a mark of algorithmic and technical thinking that is established over time. That’s why I always repeat that a good developer generally grows within the team, not alone, unless they are gifted or very experienced! To anticipate, with new developers, we do a lot of pair programming in turns. This helps to strengthen human relationships and also to align everyone and create this DNA or stamp of thought. Indeed, the second part can take me days (not the whole time, but two hours at most per day) and involves back and forth between me, the developer, the business (PO and UX team), and also the testers. Ideally, the BDD approach could save us a lot of time, but I work with what I have and adapt to the work context :).
A well-written ticket should present, in a techno-natural language, for example (GHERKIN), the simple, complex, and also possible error scenarios that may occur. One should not only think about the ideal case when writing the ticket; this will impact the review later. Until now, despite the efforts made in the team, we have not been able to identify all the cases at the time of writing the ticket that we discover during development and also during the review. For me, this is normal since software engineering is an empirical scientific work in a machine language with many dependencies and unknown side effects in advance.
In some organizations, UX review is added with the design team to ensure that we are pixel perfect. This leads us to possibly add snapshot testing at the level of our CI/CD. To ensure even before reviewing with the UX team that our code meets the expected design rendering.
Over time, we have felt that the review has improved, and most developers on the team have understood the usefulness of code review and thrive on doing it. I think the main reason is that each of us has controlled our ego and understood the benefits of being interested in this topic, which brings a lot to the product. The review allows a junior developer to level up if done with experienced and humble people.
Conclusion
The topic of code review is taboo for me in several organizations. It is rare to find tech companies that do it well with humanism. In my opinion, even people who claim to be crafters have contributed to making this process a source of pleasing their uncontrolled arrogance (there are always human beings who like to judge and show themselves better than others). I don’t claim that the review process I apply is the best. It’s my approach; there are surely better ways to do it that I am always eager to discover and apply.