Peer Code Review: Best Practices

Improve development team collaboration

Photo by Alvaro Reyes on Unsplash
My Github contribution chart

🤔 The Why

Photo by Emily Morter on Unsplash

Detecting potential bugs

Each developer has a certain perspective to check the code. The more reviewers check it, the lower the risk to discover a potential security bug on production by chance.

Improve developers coding skills

The ideal company would have only senior developers. But since these companies don’t exist, doing code reviews will upskill all the developers including picking-up new coding techniques.

Onboarding new developers

Just letting new joiners check out the codebase will help them to understand how it works faster, what is the current coding style and how the new pull request is going to affect the repository.

Knowledge sharing

If each developer only writes their code and push it without no one seeing this code and understanding abstractly how it works. Then when the time comes for this developer to leave, it will be stressful for both the team and the developer.

📝 Checklist

Photo by Kelly Sikkema on Unsplash

🧪 Tests

Added Tests

Any new code you add or amend should have tests to cover these changes. If you fixed a bug, add a test to detect if it happens again. If you add a new feature, add tests to cover its requirements and expected behavior.

Understandable Tests

They should be like ready a story. It should be written well and clean like your typical codebase with no discrimination. Your future self or coworkers will probably have to read these tests to do any codebase changes.

Match the requirements

For each task, there are expected behaviors. The tests should always match and confirm that these behaviors are valid.

Document code limitation

If there are certain environment constraints, the tests should act as documentation for them and check if an exception is thrown.

Cover failure scenarios

Adding to the above point; when an exception is thrown due to a third-party service, the tests should cover how it is handled. Expect a service might return a 4xx or 5xx errors at any point for any reason.

Performance Tests

Slow system response is what kills new applications and you should always test if your application (both frontend and backend) are responding with acceptable time and bandwidth constraints.

🏃‍♂️ Performance

Performance requirements

You should check if there are performance requirements for the added code. Will the new code take a long time to respond? will it slow down existing services? will it increase the cloud service billing costs?

Third-party services

It’s essential to check if the code will do expensive calls to the third-party services. Like if it is calling the database, is it calling it multiple times while batching in a single SQL command is possible.

Race conditions

Root for many evils in the software industry. Any new code should be reviewed and checked if it is in any way that would cause a race condition. Like when the API call depends on the user token while it might not exist yet.

Memory Leaks

It can deteriorate both the frontend and backend. The first one will freeze the browser and the second one will make your company go broke. It is famous when you use recursion. Make sure the new code can catch and avoid memory leaks.

Closing connections and streams

If the new code opens a connection to steams like databases, ports, and files, they must always close them to release these resources for new requests. Also to avoid possible database timeout or potential crash.

Caching

Check if the new code caches third-party requests when possible. You will improve the time constraints of the application and the number of maximum requests that could be handled.

🏗 Data structure

Correct data structure

This will affect the code performance and memory usage. Like for example, arrays should be used for sorted things while hashmaps or dictionaries should be used for quick search of unordered items.

Simplicity

Avoid using a complex data structure, this will make it hard for new joiners and your future self. This one would be easy to catch during a code review since you will probably question how it works from the code owner.

SOLID Principles

Any new code should follow all five SOLID principles. When reviewing code, detect any violation of them and ask the code owner for reasoning and most probably how to fix it.

  • Open-Closed Principle
  • Liskov Substitution Principle
  • Interface Segregation Principle
  • Dependency Inversion Principle

🛡 Security

MITM, CSRF, XSS…

These are a few of the most famous security vulnerablies that you should be aware of while reviewing the code. If you have no idea about them, you could check them out on YouTube or this quick tutorial

Used dependencies

Check if the used dependencies are updated and avoid potential bugs or security concerns. You can also automate this using Dependabot.

Access control

User authentication and authorization should be taken into consideration in the new code. Make sure the code uses the right token and permission to avoid potential data leakages that could potentially cost millions in fines.

Encryption

Always use encryption for transferring any sort of data, no exception. Tokens and passwords should be hashed. Always make sure HTTPS is used in the infrastructure as no justifiable reason to ever use HTTP today.

Application token secrets

If the pull request contains token secrets, they should be immediately removed from it and rotated at once.

Logging

This should be a topic on its own but in general, any action that happens due to code usage should be logged. If a resource is added, read, edited, or deleted all should be logged. With the intend that in case of bug investigation, it should be traceable.

⭐️ Quality

Codebase navigation

The main purpose is to keep everything organized and easily accessible using your editor or IDE quick search. Make sure committed code and files are well-placed in the correct place

Code inspection

There are automated tools for each language that allows you to inspect the code for potential code bugs and errors. Use automated tools to do that for every new pull request.

Unused code

Any code that is commented out or unused should not be inside the pull request and be removed. There are automated tools for this as well as plugins for your code editor.

Wrapping up

Code review might be a little bit exhausting but if it so then you are probably reading a bad code. It is essential for keeping the contributions of the whole team on track and have a maintainable and adaptable codebase.

Web Application Developer. Knowledge hungry always learning. Aspiring to become a Web Unicorn. Find me @abduvik on social platforms.

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store