On many job interviews, I got questions like “How to do code review?”, “What should you check during code review?” or “How many lines should have pull request?” I was always answering based on my experience from the last reviews. Unfortunately, I find these answers poor and insufficient. That’s why I decided to write this post.
Code review is an inseparable part of the software developer’s job. Each experienced programmer on some level of his/her career will start doing reviews. So, how to do it right? On the internet, you can find plenty of information about good code review. This topic is a little more complex than it looks like. I decided to split my answer into few smaller answers.
Goal of code review
Why do we need it at all? Can’t we just merge our code to the main branch to increase development? Some people threat code review by pointing out failures and mistakes. Is oppressing younger developers the main goal to show them where is their place in hierarchy? NO! The review is primarily the help. Working in the team we all try to win the game. If we play better and cooperate, we will be glad about the results. The end effect, thanks to code review and pair programming, causes that momentum of the team is a resultant of each team member’s knowledge. Working together we avoid situations that only few team members pull a project forward and the rest is like an anchor. The anchor is of course extreme case…
How to review the code?
When we have a pull request the easiest way is to just check files from top to bottom. From my perspective, it’s worth pulling this code to IDE to be able to check more aspects easier. Maybe our editor will reveal some things that normally you are not able to catch when reading code in the browser. That’s why I encourage you to take your time and do a review as thorough as it is possible.
To not leave this idea without a concrete action point, I prepared a short checklist.
What to check during code review?
- Check if tests are present.
- Check naming conventions.
- Take “magic numbers” into account. These are all numbers in code that mean exactly nothing to the reader.
- Functions should not have too many parameters.
- Check unnecessary imports.
- Make sure that formatting is correct
- Check if the code follows SOLID and DRY rules.
- Try to understand the code and the author’s intention.
Do we have to take everything into account?
In my opinion no. Nowadays many tasks can be automated. Doing computer work is just wasting your time. For example, thanks to tools for static analysis, we don’t have to take into account things like CodeSmells, unused parts of code, or duplicated code. Thanks to checkstyle plugins we don’t have to check i.a. length of lines or some naming conventions. Tests should in theory check code from a business requirements perspective. Btw in the case of tests quality, it’s worth to ask the question “Can I trust the person who wrote tests?”, but this is a topic for another article 😉
How bit parts of code should we push to review?
Generally, less is better. Of course, to be able to create a small pull request we should take care of the planning session to have tasks as small as possible. Story decomposition and splitting into smaller subtasks can significantly influence the size of pushed parts of code.
When I was looking for an answer on how big merge requests should be, I found this article. It says, that we shouldn’t check more than 200 – 400 lines of code at once. Beyond it, we lose our focus and we stop caching basic failings.
In pushing small pieces of code regularly lies one more superpower. By providing changes more often we see progress. We see that we do small steps forward which can improve our motivation to work. I personally feel bad and demotivated when I have a big task to do, where I cannot split my work into smaller pieces.
Benefits of code review
Learning
When you look into the code of others, you have the possibility to learn project rules and programming techniques. You can learn new approaches to solving known issues and discover completely new technologies. Code review time is the best to gather more information about good practices of creating software.
Helping to others
Commenting on pull requests you allow others from your team to use your experience. Checking other’s code is a good moment to discuss current solutions and the possibility to propose some improvements, which will make the project more maintainable.
Preventing technical debt
Technical debt is one of the most important reasons to do a code review. With all due respect to all programmers, but we shouldn’t trust anyone. I have a feeling that people are lazy by default and like to shorten the way. Without appropriate inspections project can take a bad direction.
Finding bugs on early stage
This is obvious. Providing functionalities is always cheaper when we are able to make it perfect earlier. The review allows fixing bugs before the code is deployed.
Summary
From my perspective code review gives an enormous value to everyone. The project gains quality and lowers technical debt. On the other hand, programmers gain new knowledge and experience. When I start working on a new project, code review allows me to get acquainted with the rules which the team has set. Additionally, when looking into the code of more experienced developers I can learn new things. It’s often that I find there new approached or techniques which I earlier didn’t know. It’s worth mentioning that not only the code of more experienced programmers is valuable. I love checking the junior’s code not only because I can share my knowledge. Code of beginners reveals often mistakes that I couldn’t even imagine. Thanks to such lessons I can fix bugs faster in projects that I work on.
If you like this article I invite you to like my Facebook website and to my private group. If you are a polish speaker I also want to invite you to visit my other blog.
About author
Hi,
my name is Michał. I’m software engineer. I like sharing my knowledge and ideas to help other people who struggle with technologies and design of todays IT systems or just want to learn something new.
If you want to be up to date leave your email below or leave invitation on LinkedIn, XING or Twitter.
