DEV Community

Baskarrao Dandlamudi
Baskarrao Dandlamudi

Posted on • Originally published at baskarmib.netlify.com on

What we should look for in Code Reviews

We might be having different opinions about Code Reviews. There is no one step formula to define Code Reviews. Every company and team have their own style of reviewing code.

In this post, I will be sharing my opinions about Code Review which are specific to C# and dotNet Core which can be extended to other languages were applicable. Thanks to all who responded to my tweet sharing your views on Code Review.

Common Opinions about Code Review

  • For some Code Review is about maintaining readability in application code.

  • For some Code Review is about using the right design aspects.

  • For some Code Review is about checking if the new changes are not impacting the functionality of the system.

  • For some Code Review is about following the industry standards and practices.

  • For some Code Review is about making sure performance of the application is not impacted with new changes.

  • For some Code Review is about making sure code coverage metrics are ensured properly with unit tests and integration tests.

For some Code Review is used as an approval process to merge code and exhibit authority. If this is your team do not do this.

Code Review Demystified

In my opinion Code Review can be performed by focusing into the below three zones.

  • Technical Review
  • Functional Review
  • Non-Functional Review - Architecture Characteristics Review

Technical Review

As part of technical review, we can review the technical aspects of the code ranging from design, low level implementations and look for improvements to reduce technical debt. Every team can define their own checklist which can be used before committing code to feature branch or main branch. Having a standard defined by the team, would reduce the number of hours spent on code review making the process smooth and quick and overtime this would help the developers in practicing writing application code that is clean and maintainable.

Some of these would include below.

  • Refactoring repeated logics.
  • Ensuring concepts like SOLID, DRY is followed based on use case.
  • Ensuring proper memory utilization by properly disposing objects.
  • Avoiding high cyclomatic complexity.
  • Checking for n+1 query issue in for loops.
  • Checking for object dependencies.
  • Proper handling of opening and closing of database connections.
  • Ensuring concepts like Circuit Breaker and Retry Patterns are followed where applicable.
  • Optimizing string utilizations using constants like logs across the application.
  • Optimizing async and await operations following the guidance mentioned in resources.
  • Using proper naming for methods and variables maintaining readability as per agreed standards.
  • Avoiding boxing and unboxing where applicable.
  • Unless it is required avoiding .ToList() on query results from LINQ Queries.
  • If performance is a more important concern, design your API to use arrays, List or ImmutableArray.
  • Avoiding IEnumerable, IList, and IReadOnlyList for property and method return types if we can, and do not use ImmutableList unless we are actively manipulating immutable collections.

Functional Review

We can perform functional review by making sure that our code changes do not impact any existing functionalities, by ensuring that our code is tested properly using unit tests and integration tests. This will be our first focus area as part of Functional Review.

As a common practice we can ensure that all our existing tests and new tests are getting passed before our changes are merged to main or feature branch. If we use Test Driven Development as our development methodology we can ensure that all code is tested by developers before merging the same to main or feature branch.

Non-Functional Review - Architecture Characteristics Review

Non-Functional Review is to cover areas like performance and security which are either not included as part of our requirements or functionalities. As stated in Mark Richards and Neal Ford - Fundamentals of Software Architecture book, it is good to consider requirements like performance, security, availability , reliability as architecture characteristics instead of calling them as Non-Functional Requirements.

For example, we can review aspects of code performance by comparing the metrics taken by running Benchmark tests and making sure that results are not deviated more from previous captured metrics. These can also be included in our test.

Verifying these metrics as part of code review helps us in making sure that code will not introduce major performance problems in test and production regions. We can also go an extra mile by making sure to verify secure coding practices as part of our reviews.

Closing Thoughts

A proper code review can be done when the reviewer runs codebase on their machine. Reviewing code using GitHub alone might not help because we are not executing the code or looking at the big picture on how the dependencies are used if we must review the code based on architecture characteristics.

Software development methodologies like Extreme Programming generally encourages a collective code review by the team, where the entire team can review the code and suggest improvements collectively.

If we are given an opportunity to review code it is our first chance to mentor junior or peer developers and not using them as an opportunity to criticize.

Code Reviews can be done quickly when the commits are small. Feel free to check the resource section on additional details about micro commits. Interested to know how code review is done at Microsoft, then check out the references section on details about CodeFlow. The article itself is a great read on how to improve code review process. Thanks to threddyrex for sharing this article.

Feel free to share your thoughts on different aspects that we can generally use as part of code reviews.

Related Links

Extreme Programming and Collective Code Ownership

Extreme Programming Overview

CodeFlow - Code Review Process Improvement

SOLID Design Pattern

DRY principles Overview

Async and Await Reference Guidance

ASP Net Core Guidance

dotNet Performance Tips

For and ForEach Performance Benchmark

ForEach Performance Observations

Micro-Commits

Benchmarking Csharp Code

Secure dotNet Development

Top comments (1)

Collapse
 
krishna_901 profile image
Atharva Kulkarni

Sir! How can I contact you ? I need a solution to C# code. Can you please help me?
You can mail me as a response at: atharvakulkarni55@gmail.com
Thank you!🙏