Best Practices for Code Review

Eira Tse
BigCommerce Developer Blog
4 min readMar 30, 2020

Software engineers read a lot of code in their day-to-day work, whether it be trying to understand the root cause of a particularly nasty issue or simply ensuring the quality of a peer’s work. Code reviews are one such way to uphold good standards and ensure the team is across the work that will be pushed out to production.

At BigCommerce, code review is integrated into the development process. As a company, we try to encourage sharing and growth in a safe environment. As an engineer, and fellow colleague, there is ample opportunity to learn from another, and in turn provide guidance and advice, in the form of constructive comments and polite suggestions within each pull request.

Getting Started

Let’s imagine that you are a newly-trained engineer and you have been requested to review some code.

You freeze.

Your colleague has more experience than you do and they certainly know a lot more about the codebase as well. You browse through the work of your colleague and you can’t help but admire the code that you can only dream of writing.

This is where you take advantage of their knowledge. They have the experience over you and they know the product.

  • Take a look at the code, what problem does this solution solve?
  • Follow through their code and try to understand if it does what it intends to do
  • Question their introduction of new methods and variables
  • There might even be cases where you wonder why they did X instead of Y
  • Pointing out a lack of context could necessitate documentation

Take this chance to ask ridiculous questions. You never know when you may chance upon a simple mistake your colleague easily overlooked from code overdose.

If the code is still challenging, do not be afraid to simply walk up to the author and request they pair with you; help you go through their code and have them explain their thinking and why they came up with the solution. The questions you ask could prompt the author to rethink their own solution if it appears it is too difficult to read. This will not only open your mind to the types of problems you will eventually encounter but also give you the much-needed context for when you dive into the codebase yourself.

Building Confidence

Now imagine a couple of years have gone by. You finally understand your if statements and your for loops. You can tell when a function can be written in a more succinct way or you may even make suggestions to rewrite an approach.

It is time to look beyond adding and subtracting from existing methods and begin creating and deleting classes and modules. It is time to learn how to architect a project.

  • Are you aware of the SOLID design principles? Does the code adhere to them?
  • How many design patterns do you recognise in the code? Can one take advantage of a pattern here?
  • Does your new class reside in the correct domain (according to Domain Driven Design)?
  • Are there chances to uphold the boy scout rule and clean up the code?
  • Can any performance improvements be made?

Legacy code comes with many projects you will work with. It is simply a matter of your attitude towards approaching the code and incrementally improving the product, without disturbing the years of work previous engineers have done.

This will come with exposure to a variety of different code components and builds, and perhaps even frameworks. You will realise there are multiple ways to solve a single problem. Lean on your colleagues and raise discussions when you have doubts.

Sharing Your Expertise

Now finally, imagine you have a fair amount of years under your belt. Now is your time to go that extra mile. You can branch out and start understanding the organisation and the product as a whole. You may even have ideas for improving the cultural standards of your team and the office. This is the time to impart your wisdom upon your colleagues.

  • Review code outside of your immediate team, something different that piques your interest
  • Review developer processes within your team; seek bottlenecks with these processes and brainstorm ideas for improvement
  • Mentor a junior developer — they have submitted a PR that needs a little extra substance

This will vary depending on your current work circumstances. If you are on a small team, have a look at other areas in the business. Ask for a trip down to the warehouse and watch their processes.

At BigCommerce, we have a day where we get to sit next to one of our wonderful customer representatives and watch them deal with questions and issues that you may end up fixing or implementing.

Having a greater understanding of how the business works and greater clarity in the position you are in, will provide clues to begin solving problems and making decisions around the approach of code. Eventually, these earlier decisions influence what you end up writing in your code and how you can prepare it for future iterations.

In Conclusion

BigCommerce focuses on a welcoming environment where we encourage everyone to be open with their opinions. Each engineer is at a different stage in their career and each individual will have a different view on how they would solve the problem. It is important we respect and value everyone’s opinions, observations, and suggestions.

Let code reviews be one of those safe spaces.

--

--