Tips for Effective Code Reviews

In the field of software development, code review plays a very vital role. It not only enhances the code quality, but also identifies issues before the code goes into the tester’s hand.

Throughout my development experience, I came across many amazing guidelines that can contribute to an effective code review.

So, as part of this article I’m going to list down all those points.

Naming Conventions, Data types, Sizes and Comments

• Variable names should be small but self-explanatory. Based on the language and platform, standards and casing should be followed.
• Always prefer field names using adjective or noun.
• Prefer to use PascalCasing for resource keys.
• Always prefer to name events with a verb. It’s good to think about tense while naming events, i.e. rendered, rendering, etc.
• Keep method size as small as possible. I personally prefer, size of method body of <=12 lines. So, that complete method can be seen without scrolling the screen.
• All the parameters should pass validation check before passing to any method.
• Keep class size as small as possible. If it is not possible to achieve this due to business requirement and design, then preferred way is to go for partial classes.
• Comments should present at class, interface and at method level. If logic inside a method is complex and not self-explanatory, it’s good to add few lines of comment inside the method also.
• Proper indentation plays a very big role in the readability of any source code. So, never ignore this.
• While working with expression trees, prefer anonymous types over tuples as you may have a need to rename properties.
• Avoid boxing and unboxing as much as possible. It’s always good to go with concreate data types.
• Prefer using StringBuilder over String class, whenever situation demands high concatenation operation.
• Prefer lazy loading and initialization as it would increase memory footprint. Do only when it is really required.
• Avoid extending Value Types. 
• Prefer enums over static constants.
• Avoid using enum for storing single value.

Object Construction

• Never define a public constructor in the abstract class as public constructor is useful only if an instance has to be created.
• Avoid doing much work inside constructor, apart from initializations.
• Avoid using static classes unless there is a specific need.

Object Destruction

• Make sure to take care of memory allocation and deallocation.
• Avoid using Finalize method for managed objects as it would be released automatically by garbage collector. Recommended approach would be to implement IDisposable.

Termination/Exit Criteria and Exception Handling

• There should be a single exit point for any function. Excessive use of return statements in a function body should strictly be restricted.
• Always add Default case to switch case statement.
• Always add else, if using if-elseif.
• Anyone can call your code without asking you. So, always handle NULL checks.
• Prefer Using statement to automatically cleanup resources, that implements IDisposable.
• Use Finally block to clean up resources that don’t implement IDisposable.
• Need proper exception handling with more generic exception types on top.
• Prefer to throw exceptions, iff you do not want error to be unnoticed. In other words, it is always better to avoid throwing exceptions for simple scenarios.

Serialization

• All the classes that needs to be serialized must be marked with attribute Serializable. Once class is marked as serializable, all it’s public members will automatically be serialized. For any private member to be serialized, it has to be explicitly decorated with attribute SerializeField.
• Serialize only the required members rather than serializing the complete class.
• While working with serialization, map all the properties explicitly as serialized name. It won’t break the code in case if anyone renames any property in future.
• Prefer a class or a struct over anonymous types or tuple types, for serialization.

Threading

Avoid using Thread.Abort for thread termination.

Avoid using Thread.Suspend and Thread.Resume to synchronize threads. Better way to achieve similar behavior is to use Mutex and Monitor.

Avoid synchronization as it decreases performance and maximizes the likelihood of deadlocks.

Prefer Interlocked class over lock statement.

Code Structuring

Implement proper design patterns wherever possible.

Prefer parallelism.

Make intelligent decision between Abstract class and Interface.

Do not add members to already released/shipped interface as it will create versioning problems.

Inheritance hierarchy should be chosen very carefully as badly implemented inheritance could be very dangerous. I personally avoid inheritance as much as I can, specially beyond 2 levels.

Reflection and Caching

Try to avoid reflection as much as possible.

Analyze caching scenarios very carefully as not all scenarios are suitable for caching.

Database Practices

Prefer stored proc over inline SQL queries.

Be wise while taking indexing decisions. There are some tools (Database Tuning Advisor), which also provides indexing suggestions.

Code Analysis and Coverage

Code should be error and warning free.

Using code analysis tool can help in identifying many of the common mistakes. I prefer the one, which comes with Visual Studio.

 Keep an eye on various code metrices, i.e. cyclomatic complexity, depth of inheritance, cyclomatic complexity, loc, etc. If any of these are beyond certain levels, then refactoring has to be taken up. 

A good code coverage can tell, how maintainable the code is. Hence always prefer to write unit test cases, which will also help in identifying unused and unreachable code blocks.

Last but not the least, there are many very well-known coding principles exists, which one can follow:

DRY – Don’t Repeat Yourself, KISS – Keep It Simple Stupid, Hollywood Principle – Don’t call us , we’ll call you, SOLID, YAGNI – You Are not Gonna Need It

Proper code review helps in slashing maintenance cost of the software and it should be part of day-to-day activities.

Hope you enjoyed this checklist.

Back to You

This post is just the start and there is lot more which can be added.

Please share your thoughts and comments, so that I can make this list more extensive.