Lessons I’ve learnt as a code reviewer
Code reviewing is one of the most underrated topics in software development. When done correctly, it can be a key element in the development cycle and help teams maintain a cleaner code base.
Code reviewing is one of the most underrated topics in software development. Most teams don’t even bother to do it while some do it for the sake of doing it. When done correctly, it can be a key element in the development cycle and help teams maintain a cleaner code base.
How does Ascenda do it?
Before Ascenda, I worked for 4 different companies where I worked on multiple projects and teams. Throughout this period, I did thousands of lines of code changes and created hundreds of pull requests. However in any of these companies, code reviewing was never really a mandatory part of the entire process. So personally I never really understood how code reviewing can be such a crucial step in the whole development cycle.
But in Ascenda, it is part of the whole development culture. A PR will go through rigorous amount of code reviewing before it is even taken by QA to be tested. My very first PR at Ascenda received lot of comments and suggestions. At first It was quite overwhelming and I couldn’t believe the amount of mistakes were pointed out in my PR.
This continued for few more PRs and after each review it only made me better as a programmer. Every time I opened a new PR, I minimised the mistakes I made earlier and got to a point where I started to understand the full potential of code reviewing. Then it got better when I started to review PRs of others as well.
Code reviews could reveal more about the codebase than you previously knew
As a developer, it is important to understand the code base from top to bottom. However it is not possible to familiarise with it by solely working on your own PRs. Because in a team, you will probably be working on a set of features while your colleagues work on some other features. So there is always a knowledge gap as you are not expose to the whole development of the project. But when you review PRs, you fill this gap which will help you to get a high level view of the whole project. This is an important from the team point of view too. Because this reduces the burden of knowing a certain part of the app only by a single developer. Therefore removing dependency within the team.
You can learn multiple ways to approach a problem
When you reviewing someone else’s code, you have to think from the other person’s perspective. And this person might not be thinking the same way as you do. This person might have better ways or cleverer ways of finding a solution to a particular problem. These tips collectively will help you to write more efficient code in the future.
You can teach someone else with a better approach
There is nothing more satisfying than teaching something you know to another person. When you review a PR and find out that there is a better way to approach a particular problem, you can enlighten your fellow developer about it, which would ultimately beneficial for the entire team. I think this is one of the best ways to improve the team dynamics within a development team especially if there are new members in the team.
You can reduce the effort which would be taken by QA and therefore increase overall efficiency of the team
Normally the ratio between developers and QA in a software team is 3:1 Which means usually in the middle of a Sprint, QA work is overloaded by lot of features/bugs which need to be tested. The worst thing about this is sometimes us developers tend to do silly mistakes. When there is no proper code review in place, these issues are detected at the very early stage of QA testing and are sent back to the developer almost instantly. Let me explain this more using an example.
Scenario: Imagine there is a task which is basically adding a banner text within the app in few different places. Let's assume we need to add this in landing page account page and in the email template.
1) Developer A, completes the task, creates a PR.
2) Developer B approves the PR without conducting proper code review.
3) PR is merged to staging and deployed.
4) Task is sent to QA, finds out that the changes are only made in landing page and accounts page but not in email template.
5) QA labels it as needs fix. Send back to developer A who is now working on some other task.
6) Developer A again switches back to this PR and fixes it. Again step 2 , 3 happens.
7) QA finds out that this time the developer A forgot to change the colour of the text in the email as the task says. And it was again labeled as needs fix and is sent back to the developer A and the cycle continues.
If you look at above scenario , you could see there is a cycle going on. Imagine if developer B had conducted a proper code review in the first place, He could’ve seen this issue almost immediately and informed developer A. This would’ve minimised the time and the roundtrips between the developer and the QA. Also this would’ve minimised the context switch happens between the cycles for developer A. This is a major point holding back teams from developing and shipping features as per original estimation and schedule. Teams could overcome this with proper code reviews in place.
You could avoid unexpected production issues by identifying edge cases in early development
This is I think what code reviews are meant to be in the first place. Usually when you implement something complex, you tend to miss edge cases. Sometimes even QAs don’t find it because well, those are hidden in the code and overlooked during testing. Especially if they are related to security. But these edge cases could pop up during proper code reviews. It makes more sense if you imagine code reviews are your first line of defence against these nasty production bugs.
Code reviews help identify performance bottlenecks and memory leaks
Imagine a simple scenario where there is a feature which is done using 2 loops. QA sign this off because the feature works as per the requirement. But what the QA doesn’t know is this feature could be done using a single loop in the implementation and therefore reducing the complexity from O(N^2) to O(N). This wouldn’t be a problem when QA tested this against the staging data which is a small dataset compare to production. But when size of N increases (in production) it could pose a significant performance issue which could’ve been eliminated at the very early stage of the development.
Code reviews help identify duplicate code and enforce re-usability
When you work in large code base, It is imperative that the teams follow one of key principles in software development which is re-usability. Otherwise you will end up with lot of duplicate code which will lead to maintenance hell. Code duplication is not something a QA could find out. So even if the PR contains duplicate code but if it runs according to the spec, then it will be the signed off by QA. But this is bad for the codebase. These can be only unearthed during proper code reviews.
Wrapping things up
Code reviewing should be embraced as part of the development culture and there is no doubt about it. You could avoid code smells, performance issues and code duplications if proper code reviews put in place. Also it is fun to see how others solve a common problem in a different way. Most importantly it teaches to be a better developer.