In my ~15 years as a software engineer/lead in global tech corporations, I made 100s if not 1000s of Code Review s. I captured here a list of common patterns in code reviews holding them up from completion and causing 10+, sometimes 20+ review iterations before the code can be checked in.
I expect these to be beneficial especially for the junior developers but also experienced developers as a baseline and a refresher. Like all principles and best practices in software engineering discipline, there could be cases not to follow these, but the reason should be a good one and thought through. So lets dive in.
1. Parameter Validation:
All public methods and constructors need to have parameter validation. Even better, when invalid parameters are detected, parameter name and value, should be logged.
2. Dependencies and Dependency Injection:
Take dependencies as Interfaces (not concrete classes) through class constructors explicitly. That allows your components to be testable as you can easily pass a mock implementation of the interface to your class.
Use a IoC container for object composition. This makes it easy to manage dependencies in a central location (composition root).
3. Logging:
Logging is super important. All unexpected errors (and high informational value non-errors) should be logged with enough logging metadata and information to allow us properly classify, diagnose and root cause issues happening with the code we wrote when it is used in the wild in production.
Wrap your code with performance counters to be able to monitor its metrics in Production.
4. Asynchronous Code
All I/O calls (service call, disc I/O read etc.) should be async. All async methods should be awaited ie. await FooAsync(); instead of making a blocking wait on them ie. FooAsync.Wait() , FooAsync.Result.
Making a blocking wait on an async method defeats the whole purpose of asynchrony and introduces potential deadlock issues.
Making synchronuous calls on I/O requests unnecessarily blocks the Core while I/O request is executed and introduces scalability, perf bottlenecks which can be hard to fix after the fact once these issues accumulate over time.
5. Parallel Processing
For the code that does not need to be sequentially run, leverage parallelism in your code but balance this with an upper bound on Max Degree of Parallelism. Unbounded parallelism/ over parallelism also may result in side effects.
If in doubt, measure the execution time and CPU of your code before and after and optimize the level of parallelism.
6. Code Reuse
Do not reinvent the wheel and contribute to shared code. Search code locations in your team’s repository that contains shared, reusable code to see if the api you want to implement exists, if it does use it, if not then add it to a suitable shared project so it can be reused by other engineers/teams if it has potential benefit for other teams.
7. Small Code Reviews
Send small code reviews that can ideally be completed within 5 iterations. Talk through unclear comments with the reviewer in person.
8. Engage in Design Reviews Early On
Major software and system level architectural changes should go through a Design Review before the code review is sent. Before coding starts.
Hope this helps 🙂 Happy coding.
doguarslan 