The significance of reviewing code
The importance of thoroughly reviewing code and what kind of impact it can have.
I know reviewing other people’s code can be time consuming and feel a bit boring; I’ve been there. There have been so many times where I sit at my desk, looking at my monitor, thinking to myself “Ok ok ok ok, looks good… I don’t really care about these unit tests…oooh this function is big and complex. I’ll assume it works … Ok! It all looks good. Approved!” This was a mistake I was constantly making. I didn’t realize I was indirectly affecting my team by lowering the standard and directly slowing my growth by not engaging and learning from other people’s code. If you’re thinking “How is that really a mistake? What’s the big deal?”. Well, let me explain.
What are the benefits and importance of code reviews?
This goes beyond the obvious reasons of catching bugs and issues. The more I’ve worked in my field, the more I’ve realized the benefits extend far beyond just me and the developer whose code I’m reviewing. I will break this down by stakeholder.
The Team and Organization
A lot of our time is concerned about the daily tasks that lie in front of us and we don’t think too much about how our work affects the bigger picture. We know shipping code with bugs is a bad thing but have you really thought about what kind of cost that is to your organization? Understand that retroactively fixing bugs is a very costly process. The money spent for your team’s time to generate tickets, pause development on a feature, investigate the bug, fix it, QA and verify the fix, and eventually creating a patch release is way more costly than if it were caught and addressed during a pull request review. You and your team will be able to spend more time working on new features rather than time being spent on fixing a bug or rearchitecting the code.
The Reviewee
Who’s the reviewee? The poor soul who’s submitting their code to be ridiculed and criticized by their peers. This is how I always felt. It was scary for me to put my work out there to be viewed by other engineers who were more senior and experienced. With time, I’ve realized that I appreciated the feedback from others. I’ve learned that people can see things from a unique perspective that I can learn from. Some engineer who has a lot of experience writing error handling around HTTP requests will probably have a perspective that can enhance the work I did. That’s eventually how I learned to be more open and welcoming to the idea of others reviewing my code. It’s an opportunity for you to learn. You can grow from other people’s experiences. Pull requests are a great forum to address your code with critical feedback. It’s a place where you take in suggestions from people who see potential issues that you aren’t able to perceive yet. It’s also to address questions and inform your team the reasons for your implementation and designs. It’s where the debate begins on why your code will or will not work to meet the project requirements and meet the team standards.
I know you might not be a fan of discussing code over a pull request. There are drawbacks to it that we all experience. For one, it’s highly impersonal. It can create tension amongst team members because it’s difficult to determine their tone. Sometimes, you may feel like they’re picking on you because they always have requests for changes and issues with your code. Also, the rate of information being transferred is restricted by bandwidth of communication. I know how slow the process can be to go back and forth with your teammates. It’s like communicating through email chains until there is finally a general consensus. It would absolutely be faster if you could just have the lead engineer be by your side, looking at your code and talking it through with you while making changes until it’s ready. This is a faster strategy and I wouldn’t blame you for preferring it this way. However, everyone else just missed out on this valuable interaction. During that interaction with your lead engineer, you may have exchanged information on why you wrote your code that way, your lead may have spilled some knowledge on you, you may have introduced a new concept to your lead and taught them something new. Everyone missed out and there is no paper trail to what was exchanged in that interaction.
It is so important for people to give their feedback on code reviews. It is our duty as engineers to share our knowledge, inform, and improve our code. At the end of the day, these reviews are investments in your growth. Your team is making sure that you’re getting the feedback you need to grow into the self sufficient engineer that they need.
The Reviewer
This is a critical role that we must play when becoming a software engineer. It’s a skill that can’t be taught in school but is a necessity to have when working on a team. It’s part of your responsibilities to review code whether you are an intern working your first job or are a principal engineer who has been writing code with multiple teams over a long career. We all must review our team members code regardless of the experience we have. You, the intern, need to ask questions, run the code yourself in order to learn and elevate your skills. It is possible that you may bring a fresh perspective that might educate the rest of the team. And you, the principal engineer, need to explain, challenge others to think, give them the tools to grow so that you lead a team to success.
I believe that bad code in a codebase isn’t solely the fault of the author who wrote that snippet of code, but also the other members of the team’s fault. I know this comes off a bit absurd to make such a bold statement. When I first began to review code, I had no idea what I was supposed to do. I couldn’t tell if I was supposed to just check for misspelled variables or just read the code and approve it when I was done reading it. I hated it. I would wait for other members on my team to review it so that I didn’t have to. This lazy behavior was bad for me. I fell behind in parts of the code base and didn’t know how certain components worked. Later on, when I had to go into the unfamiliar parts of the code, I would see the work and decisions that made their way into our code and thought to myself, “Why is this here? Why was it written like this? I think this can be written differently so that it is easier to test”. I missed my opportunity to make suggestions to improve the code before we merged it in. I failed at my responsibility of being a contributing member to my team. This is why I believe that catching concerning snippets of code isn’t completely the authors fault but mine as well. If I know better, then I need to review code and voice my concerns.
Additional Benefits
What are the benefits I gain from reviewing code? Well it depends. As a junior engineer, one obvious answer is learning the code base. I know it can be tedious trying to look at a file to see what it is doing and then seeing what the behavior of the changes are. This can take a while but it’s expected of you to understand how the whole code base works, not just the tasks that are assigned to you. Another benefit as a junior engineer is that you learn new techniques and design patterns. Reviewing code that isn’t written by you allows you to see the good practices other people have picked up with their years of experience. You gain a better understanding of how to think about code at a higher level. You develop a mental map of all the components that exist, and what is currently being worked on by other team members. Developing this higher level of thinking is what eventually will allow you to start thinking about project architecture and design. With time, you will form your opinions and good practices on DOs and DON’Ts around certain types of software projects.
As a senior or lead engineer, the obvious answer is you’re making sure that you use your knowledge and experience to catch potential bugs. But it’s more than that. You want to invest in your team. Your review is a way to educate less experienced engineers. You can answer questions about cool code snippets that you learned through your experience. You are there to show good coding practices you’ve picked up and explain why they should avoid certain habits. This effort is to lead your team to become more self-sufficient, independent engineers who will eventually be able to grow into experienced engineers. This intangible investment is to create an efficient team that can produce something great.
How to review pull requests?
This is a really hard question to answer. There’s not a single formula or algorithm you can apply to reviewing code. It all depends on what the objective of the project is, what kind of software you are writing, and so many other factors. However, I do have a list of guidelines and suggestions I’ve accumulated over time. The gist of what I do when I look at someone’s pull request is to play a sequence of thought exercises in my head. I know that sounds weird but let me elaborate on what I mean.
First thing I do is pull down the branch and look at the code through my editor. It’s easier for my eyes to understand code in the environment that I write code in. I know there are certain things I might miss when looking at the diff comparison in Github. I make sure I understand what the expected values are going to be during runtime. I’ll run through the code in my mind line by line as if my mind is the debugger. At each line is where I run through my thought exercises and try to break the code anyway I can. Some common situations I encounter are:
Changes to functionality
If I’m looking at some code that modifies existing functionality, I’ll ask myself questions such as “Are they misusing the original function by making this change? What was the purpose of that function before this change? Why are they changing it now? What are they trying to accomplish with this change? Should this be modified or does it need to be made into a new function or component?”. These are the questions you should be challenging yourself with. Don’t just try to evaluate if this modified code works or not. These questions are made to train your mind to start looking at code at a higher level. It will help develop your architectural skills. You will learn to write code that is organized and well planned for future problems that you will be trying to solve.
Bug fixes
Bug fixes are usually the easiest pull requests for us to review. I bet you’d probably jump on the opportunity to review a bug fix because it was only a one or two line change. We think it’s easy but we’re not doing our part if that’s how we view it. You should be asking yourself “Why did this bug exist in the first place? Does this fix actually fix the root problem or is it just patching the problem for this component?“. Sometimes it can get really complex. If you are asking how could this have happened? Was it just because there was a lapse in judgement when that was first written and we may have missed that simple bug or is it a manifestation of a deeper problem? Does this now uncover an issue with how we designed this component? Is this design not going to work for the future features we want to support? We sometimes overlook bugs and assume that they can be easy to fix but these questions need to be asked. You need to constantly be on your toes and ready to adjust for obstacles that come your way. Bugs can sometimes be indicators that an adjustment needs to be made before too much tech-debt has been accumulated. You should always try to be thinking big picture even when you’re dealing with a simple bug fix.
Use cases
The last strategy I use for almost any kind of review are simulated use cases. You should be trying to break the code that lies in front of you. Think about the use cases that you know needs to be covered, but also potential use cases that are planned, and use cases that you never really anticipated. These are more thought exercises that have helped me grow as an engineer. You begin to look at your own code the same way and try to break it as you write it. You’ll eventually become natural at this. You’ll be able to plan features in detail and make minimal mistakes because you have the mental capacity to understand all the different dimensions of your proposed design. You will have thought through all the possibilities of your design. When the time comes to execute, the implementation will be flawless.
Conclusion
I hope this has opened you to a new perspective on reviewing pull requests. It will take time and experience to view it as more than just looking at code. However, acknowledging that there is more to it right now is a good starting point. If you start to think about it more positively, you will change the trajectory of your professional career. From my perspective, it has drastically improved my skills as an engineer and I have witnessed it rubbing off on my teammates. Sincerely reviewing code can seem like a tedious chore but I’ve learned that it has helped with my growth and elevate my standards as an engineer. It has a profound effect that can radiate from you to so many people around you.