Ajay Pandey is the director of technology development at a prominent fintech company.
One of the best engineering practices I experienced in my career has nothing to do with learning a particular technology, framework or language. It’s code review. Automated tools can detect bugs in your code, check it against preset coding standards and offer you suggestions, but they won’t replace your engineering judgment. A code review isn’t just a quality gate you need to pass before merging your code; it allows you to validate the design of your program and make it more sustainable.
It becomes especially important in the context of modern C++, where you need to have full control over memory, concurrency and the performance of your application.
Start With The Problem, Not The Implementation
One of the biggest mistakes made in code reviews is starting to discuss the implementation without properly understanding the problem that should be solved.
Make sure you understand the motivation behind the solution and why this particular approach was chosen. Quite often, the source of problems in production isn’t the wrong implementation but the incorrect design of a component.
Evaluate The Architecture, Not Just The Syntax
Good reviewers go beyond looking at each function and think about how the newly introduced code interacts with other parts of the system. Failures in production happen quite often due to tightly coupled components, duplications of functionality or vague interfaces. Proper review requires you to make sure responsibilities are clearly divided between the components, abstraction layers aren’t broken and new components integrate seamlessly with the existing design.
Investing a couple of minutes on discussing the architecture of the system during review is much cheaper than spending many weeks on refactoring the system in the future.
Verify Ownership And Proper Resource Management
There are many advanced tools in modern C++, such as RAII, smart pointers and move semantics, which help with resource management. But a reviewer still needs to check if the ownership of the objects is clear.
Checking if the code uses std::unique_ptr or std::shared_ptr is only part of a good review. A piece of code may look technically correct but leave doubts about the ownership of the objects. And it can lead to problems during maintenance of the system several years later.
Review For Production, Not Just The Ideal Path
An application never fails under ideal conditions. Applications start failing when there’s a spike in traffic or something happens to the dependencies of your system.
So, a good reviewer thinks about how the application will behave in real conditions when a dependency becomes unavailable, a queue starts growing unexpectedly and retries are exhausted. Does the application log failures with all the context information needed to fix the problem?
Consideration of different production scenarios in the course of the review often uncovers problems that weren’t revealed during functional testing.
Pay Special Attention To Concurrency
Concurrency needs to be reviewed carefully because race conditions and deadlocks can be quite hard to spot until the system is working under production conditions.
Reviewers need to examine synchronization mechanisms, shared states, locking granularities and thread safety. But they also need to decide if several threads are really needed, as simplification of the concurrency may both increase reliability and streamline maintenance.
Concurrency should be always considered an architectural choice in C++.
Keep Balance Between Performance And Maintainability
Performance is one of the main advantages of C++. But optimization needs to be thought out and planned.
Reviewers should consider the complexity of algorithms and avoid unnecessary allocations and redundant copies, but make sure not to sacrifice the readability and understandability of the code. The most maintainable solution is usually the best choice unless the performance data says something else.
Think About The Next Engineer Who Will Work With The Code
Most software spends way more time being maintained than written. So, the reviewer needs to evaluate whether future engineers will be able to understand and modify the code easily.
Straightforward naming, control flow and well-designed abstractions usually give more long-term benefits than tricky code. One simple question can help in this case: Will another engineer be able to understand what was done here without going through detailed explanation? If the answer is no, then additional work is probably needed.
Review Tests With Equal Care
Code reviews should cover not only the implementation of a feature but also tests written for the implementation.
A good test set should show that the implementation works as expected in normal conditions and all possible boundary cases, including error and regression cases. This gives confidence that the feature is implemented correctly and any change won’t introduce defects into the implementation.
A well-designed test is usually the best documentation for software.
Learn Through Constructive Feedback
The best reviews aren’t one-sided critiques but discussions. Instead of just pointing out problems, the experienced reviewer explains his suggestions and invites a conversation.
Questions like “Can this ownership model be simplified?” or “How would this work with several concurrent threads accessing it?” can help authors think deeply and allow additional learning for both sides of the review.
This kind of discussion increases the engineering capabilities of the whole team over time.
Conclusion: Approval Means Responsibility
Every approval of a pull request means more than just passing code quality gates. It confirms that the implemented solution addresses the right problem, follows system architecture, works reliably in production and is maintainable in the future.
Automated tools will help increase the quality of code by following formatting rules and spotting typical defects, but they can’t evaluate architectural decisions, system resilience and maintainability. That responsibility lies on the shoulders of experienced engineers.
Forbes Technology Council is an invitation-only community for world-class CIOs, CTOs and technology executives. Do I qualify?