10 tips to make a good Code Review
Over my 4 years working as a Software Developer I have made hundreds of code reviews and this recurring practice in my day-to-day motivated me to create some kind of checklist of what I must evaluate on each pull request in order to say if some code is good or not to go to production environment.
This article does not intend to be a “bible” of how to make a code review, since I know that there is a lot of other practices made by other developers that are as important as the ones I’m going to list, but it intends to be a basic guide for people that are starting to review code from their teams.
With that being said, these are my tips of how to make a good code review:
1.The code must work
It is pointless having a well-written and beautiful code that does not work. Therefore, the first thing that I like to evaluate when reviewing code is whether the code works or not, since if the developer who submitted the code didn’t understand exactly how the code should work, all the other necessary changes will not have value.
Besides, every software developer knows that developing software not always is a bed of rosed and sometimes some technical debts have to be accepted in order to deliver value to customers faster. So, in these cases, the focus of the code review must be at the proper functioning of the code more than any other aspect.
2. The code must follow coding good practices
The coding good practices standardize your code, so it is more understandable and anyone can read and understand it.
There are some popular global good practices that are indepedent of the programing language, such as DRY (Don’t repeat yourself), SOLID, KISS (Keep it simple, Stupid!), YAGNI (You Aren’t Gonna Need It), commenting not obvious lines of code, not using magical numbers, among others.
However, there are other good practices that are intrinsic to the programming language, so code must also be evalatued accordingly. These pratices are usually related to naming conventions of variable and classes, indentation, line breaks, code documentation, member declarations, syntax, initialization of members among other aspects.
Reviewing code taking account of these good practices will help you from the future and other people who might come to get in contact with your code.
3. The code must follow the application’s pattern
In most cases, the code will be submitted to review after an architecture for the application is already chosen or at least minimally discussed.
Therefore, in order to keep the code organized it is important that every developer follows the existing organization of the code, so it is not created many names that have the same meaning.
For example, let’s suppose that the application being reviewed has a folder called /utils and it is where generic functions that can be used throughout the application are placed. In this case, functions that have the same objective must be placed in the same directory so the code is kept organized and concise.
Of course, if the submitted code is a code refactoring, it is expected that some organization and naming may change. In this case, it is important that the whole team agrees with the new nomenclatures that might be being suggested.
4. Logging also deserves attention
When builiding software, the scenarios where things go wrong must also be considered, since you need to be able to find out what happened in production environment. There are some ways to do so, one of the best ones is the use of logging.
When reviewing code, make sure that the logging messages are coherent and understandable in a manner that it is easy to understand which process was executed. Besides, in case of exceptions, make sure that logging shows you exactly what happened and where, so the debuggings can be more precise.
Also, remember that when the subject is application’s logging the thought is not always “the more the better”. If logging is still written in a text file, soon you might have problems with storage. If you send the logging to some platform such as Splunk, Kibana or DataDog, you might have some problems with costs.
Therefore, make sure that only the essential is being logged and it will be easier and cheaper to use it to create intelligence.
5. Test your code
Depending on how the application was coded, it can be pretty hard to test it, making it easier to deploy bugs in production environment. Therefore, the code review must guarantee that the code is testable and has relevant tests.
The wrong use of some strucutures such as static methods, abstract classes and global state and the lack of dependency injection may make the code less testable, so it is important to realize if they are being used correctly.
Moreover, many people worry too much about code coverage and less about the relevance of the performed tests. Code coverage is important, but keep focus on the many scenarios that the tests must cover, since a same code block may have many possible behaviors, therefore demanding many different tests.
6. Do not forget about the infrastructure
The application’s infrastructure must also be considered when reviewing code, since a poorly planned implementation may cause a malfunction of the code because of impacts in infrastructure.
For example, imagining that the application is running in a container, it is expected to have less CPU and RAM memory than a virtual machine. Therefore, it is encouraged even more that it is made a better usage of resources. Under this circunstance, if the implementation to be reviewed involves a new endpoint that gets some data, it must be considered how many rows the database query can return. If it is expected to return a maximum of 10 rows because it gets data from a domain table, there is not a reason to worry, but if it may return thousands or millions of rows, a paginated endpoint might be a good ideia in order not to use too much RAM memory.
Thinking more about the database environment, imagine a new developed feature that runs a new query in database and receives many connections, but its homologation was made with a few rows to be queried, when actually in production enviroment it can process millions of rows. In this case, because a query was not tested in a real life scenario, it can use most of the database’s server resources and impact other queries from other applications.
7. Watch the metrics of your application
Monitoring may not be the first priority when an application is being developed to deliver value to the customer faster, since it may involve extra operational and monetary costs. However, at some point of the software’s life cycle parameters such as RAM memory, throughput, availability, elapsed time of processes, errors, processed volume, among others, will have to be monitored in order to keep the system up, alert people when something is not right or only visualize data.
Therefore, it is important to check if the developed code is monitorable, that is to say that the code generates data that can be used to create metrics and intelligence. The way that data will be consumed will be different depending on the monitoring platform you are using (New Relic, Splunk, Zabbix, Grafana, etc), so it may be necessary that you know a little about the chosen platform.
All in all, make sure that the code creates interesting data that can be relevant to your monitoring, so you will know that a problem happened before your customer.
8. Invest in documentation
The process of developing software is not just about code, it also involves documentation. It may be UML diagrams, business requirements, architecural specification or some other type of documentation, but documentation is a very important part of software’s life cycle, since it avoids rework, records old decisions, assists in the improvement of the application and disseminates knowledge.
I know, it is hard to keep documentations up to date, but the effort to do so will pay off in the long term.
9. Avoid reviewing a lot of changes at once
When a new feature involves a lot of changes it is common that a lot of code is submitted to code review. Who has never reviewed a pull request with more than 100 files?
The ideal approach is to improve the planning so big changes can be broken into small deliverables, but if it is strictly necessary that a lot of changes is made at once, ask the developer to at least create many pull requests that separate the change in more parts, so they can be reviewed individually.
This suggested approach will help you to organize the review and be more assertive.
10. Allow the whole team to give their opinion
When reviewing code, do not try to do things your way, promote discussions among the team. Justify the reasons why the changes are being requested, but also ask the team for their opinion, good insights can come from this and it creates a good opportunity for everyone to learn something new.
Developing software is a collective work, so the whole team needs be part of it.
Well, I’m sure that there are many other aspects in code that can be reviewed, it really depends a lot on the developer and the patterns that each company creates. However, I hope these ten tips can be a basic guideline to help you to get better at code review.
Lastly, it is important to say that these tips are based upon ten lessons that I learned by coding and reviewing a lot of code, which I try to keep in mind in ever code review:
- The main objective of developing software is solving problems, so your code must do so
- Good practices exist for a reason, follow them
- The name of things is important
- A good logging can save you a lot of time
- Test every scenario, so you know exactly what is going to production environment
- When developing software, every little detail of the environment must be considered
- Know about the problems of your application before your customer
- Documentation is not a waste of time
- Keep pull requests as small as possible
- Reviewing code is not just about code, but also about people
