Today I will leave technical stuff and will focus on the process of software development itself. Many companies thats treat code quality seriously usually introduce a code review practice. I have personally done code reviews in my professional “career” for a long time. That’s why I have some experience and thoughts about it. This is the reason why I decided to write this post and share it with you.
Code review? What is it?
For those of you who have never touched on this topic or heard about it and are not sure what is it, I have a short definition (from wikipedia):
Code review is systematic examination (sometimes referred to as peer review) of computer source code. It is intended to find mistakes overlooked in the initial development phase, improving the overall quality of software. Reviews are done in various forms such as pair programming, informal walkthroughs, and formal inspections.
So, using different words – a code review is something that is done after we finish the implementation of some part of a functionality, usually before committing or merging the pull request, and should be shown to another member of the team. If the review is positive and our colleague approves it, we can commit/merge. Otherwise we have to amend the code in accordance with the remarks and ask for a review again.
For someone who has never had to deal with a code review, the above definition might be kinda scary. I don’t think anyone likes to be judged and they could take it personally. But, if you understand the goals of this practice, you can agree that it is one of the most useful tools that help us to create great software.
Ok, so why is code review worth doing?
At the beginning of this article I mentioned code quality… I think that this is the most obvious. If someone, preferably more experienced than you, looks at your “scribble”, they may find mistakes. They will also show you many places where a better solution can be used. Additionally they can find the places in your code where you didn’t follow the best coding practices. Thanks to this, after we correct/refactor our code, it will be much better than before.
Getting familiar with other parts of the project
Another advantage to doing code review before every commit or pull request merge (I think that’s what it should look like), is that other team members can familiarize themselves with the whole project. Even with the parts they have never seen before. Additionally, many times you can be stuck on some part of the system (especially a huge one) for a long time. That’s why it is good to break away from time to time and take a look at something different.
The last reason related to my first argument, is that it is a good occasion to learn from others. Especially from the most experienced of them. And this is learning on many levels. You can familiarize yourself with practical uses of design patterns (when the reviewer points out that something can be done in a better way). Besides, you can learn a completely new technology which you have never heard about. You can also even learn such trivial things like new shortcuts in your IDE. Often, when I do a code review (or someone does it for me), I hear: “oh, how did you do that? I didn’t know such a shortcut exists!!!”.
I know that there are many more reasons why it’s worth doing a code review. These are my first thoughts on this subject. You can share your thoughts in the comments if you like 😉
Now that we now why it is good to introduce a code review practice, I think it is worth mentioning how to do it well. There are many schools and approaches to this subject… Some are too lazy and some are not attentive enough to detail. Others focus on details too much and are too nitpick. I think that we should strike the right balance between these two approaches. An overly sketchy code review can’t provide sufficient quality. On the other hand, an overly detailed code review might delay the finish of the task. This may result in the danger that you won’t provide all the planned functionality. I have had such situations in my career: half a day of development, a code-review and then two days of fixes 😉 I think it’s important that other team members, including PMs and Scrum Masters, notice these situations and react properly.
Things to check during a code review
Those were my thoughts about code reviews. I think that now is a good time to show you a list of things to check during a good, in my opinion, code review:
- Logical mistakes – sometimes we make simple mistakes, for example, comparing values in the
ifstatement. The code works for one testing path and it seems that everything is OK. All you need is to show it to someone else and they will find it immediately.
- Naming, code readability – writing a code which speaks for itself is very important for further maintenance. That’s why it is worth checking if all variable/function names describe their purpose well. You should also check the code style, etc. If you use tools which help to keep a good code style (e.g. ESLint), check the output.
- Code which potentially needs refactoring. If you see a function which is longer than three screen heights, I think it’s a natural candidate for refactoring. The same if you see an
ifstatement with a bazilion
else'sor if you see a “callback hell”. These are only the most obvious examples. Everything depends on the code so be attentive while doing a code review.
- Comments – this, of course, depends on the approach in your project. If you expect in your project that, for example, comments for every class method, you should check it during the code review. Some teams use appropriate tools to add such comments automagically. Many times the resulting comments are dumb 😉 You should also check if all the comments make sense.
- Unit tests – first of all, check if they are present. 😉 If so, check if they cover 100% of the functionality. At the end, check what they are really testing –
expect(true).toBe(true);is not a good test. 😉
- Computational complexity / performance – many times you can see at a glance that the performance of the code you are looking at will be poor. It is better to point it out ASAP instead of waiting for the performance tests phase.
Same as before: there are probably many more points to check during a code review. Please share them in the comments if you like!
I think that a good code review is key to keeping a good quality of code. It is also significant in improving developers’ skills. I also think that everyone should use common sense when doing a code review. No one should be afraid of code reviews – it should be an opportunity to learn and to praise someone for great work. How does it look like in your company?