Cloud, Microsoft stack, BizTalk Server, MEAN stack, LAMP stack, MSBI: February 2017

This code review checklist also helps the code reviewers and software developers (during self code review) to gain expertise in the code review process, as these points are easy to remember and follow during the code review process.

Let’s first begin with the basic code review checklist and later move on to the detailed code review checklist.

Basic Code Review Checklist

Let’s discuss about the basic code review checklist, which can be very handy if you are a beginner in code reviews and/or during initial code reviews.

While reviewing the code, ask yourself the following basic questions:

1. Am I able to understand the code easily?

2. Is the code written following the coding standards/guidelines?

3. Is the same code duplicated more than twice?

4. Can I unit test / debug the code easily to find the root cause?

5. Is this function or class too big? If yes, is the function or class having too many responsibilities?

If you feel that the answer is not satisfactory to any of the above questions, then you can suggest/recommend code changes.

Detailed Code Review Checklist

The following code review checklist gives an idea about the various aspects you need to consider while reviewing the code:

1. Code formatting

While going through the code, check the code formatting to improve readability and ensure that there are no blockers:

a) Use alignments (left margin), proper white space. Also ensure that code block starting point and ending point are easily identifiable.

b) Ensure that proper naming conventions (Pascal, CamelCase etc.) have been followed.

c) Code should fit in the standard 14 inch laptop screen. There shouldn’t be a need to scroll horizontally to view the code. In a 21 inch monitor, other windows (toolbox, properties etc.) can be opened while modifying code, so always write code keeping in view a 14 inch monitor.

d) Remove the commented code as this is always a blocker, while going through the code. Commented code can be obtained from Source Control (like SVN), if required.

2. Architecture

a) The code should follow the defined architecture.

1. Separation of Concerns followed

· Split into multiple layers and tiers as per requirements (Presentation, Business and Data layers).

· Split into respective files (HTML, JavaScript and CSS).

2. Code is in sync with existing code patterns/technologies.

3. Design patterns: Use appropriate design pattern (if it helps), after completely understanding the problem and context.

3. Coding best practices

1. No hard coding, use constants/configuration values.

2. Group similar values under an enumeration (enum).

3. Comments – Do not write comments for what you are doing, instead write comments on why you are doing. Specify about any hacks, workaround and temporary fixes. Additionally, mention pending tasks in your to-do comments, which can be tracked easily.

4. Avoid multiple if/else blocks.

5. Use framework features, wherever possible instead of writing custom code.

4. Non Functional requirements

a) Maintainability (Supportability) – The application should require the least amount of effort to support in near future. It should be easy to identify and fix a defect.

1. Readability: Code should be self-explanatory. Get a feel of story reading, while going through the code. Use appropriate name for variables, functions and classes. If you are taking more time to understand the code, then either code needs refactoring or at least comments have to be written to make it clear.

2. Testability: The code should be easy to test. Refactor into a separate function (if required). Use interfaces while talking to other layers, as interfaces can be mocked easily. Try to avoid static functions, singleton classes as these are not easily testable by mocks.

3. Debuggability: Provide support to log the flow of control, parameter data and exception details to find the root cause easily. If you are using Log4Net like component then add support for database logging also, as querying the log table is easy.

4. Configurability: Keep the configurable values in place (XML file, database table) so that no code changes are required, if the data is changed frequently.

b) Reusability

1. DRY (Do not Repeat Yourself) principle: The same code should not be repeated more than twice.

2. Consider reusable services, functions and components.

3. Consider generic functions and classes.

c) Reliability – Exception handling and cleanup (dispose) resources.

d) Extensibility – Easy to add enhancements with minimal changes to the existing code. One component should be easily replaceable by a better component.

e) Security – Authentication, authorization, input data validation against security threats such as SQL injections and Cross Site Scripting (XSS), encrypting the sensitive data (passwords, credit card information etc.)

f) Performance

1. Use a data type that best suits the needs such as StringBuilder, generic collection classes.

2. Lazy loading, asynchronous and parallel processing.

3. Caching and session/application data.

g) Scalability – Consider if it supports a large user base/data? Can this be deployed into web farms?

h) Usability – Put yourself in the shoes of a end-user and ascertain, if the user interface/API is easy to understand and use. If you are not convinced with the user interface design, then start discussing your ideas with the business analyst.

5. Object-Oriented Analysis and Design (OOAD) Principles

1. Single Responsibility Principle (SRS): Do not place more than one responsibility into a single class or function, refactor into separate classes and functions.

2. Open Closed Principle: While adding new functionality, existing code should not be modified. New functionality should be written in new classes and functions.

3. Liskov substitutability principle: The child class should not change the behavior (meaning) of the parent class. The child class can be used as a substitute for a base class.

4. Interface segregation: Do not create lengthy interfaces, instead split them into smaller interfaces based on the functionality. The interface should not contain any dependencies (parameters), which are not required for the expected functionality.

5. Dependency Injection: Do not hardcode the dependencies, instead inject them.

In most cases the principles are interrelated, following one principle automatically satisfies other principles. For e.g: if the ‘Single Responsibility Principle’ is followed, then Reusability and Testability will automatically increase.

In a few cases, one requirement may contradict with other requirement. So need to trade-off based on the importance of the weight-age, e.g. Performance vs Security. Too many checks and logging at multiple layers (UI, Middle tier, Database) would decrease the performance of an application. But few applications, especially relating to finance and banking require multiple checks, audit logging etc. So it is ok to compromise a little on performance to provide enhanced security.

Checklist

Make sure that there shouldn't be any project warnings.
It will be much better if Code Analysis is performed on a project (with all Microsoft Rules enabled) and then remove the warnings.
All unused usings need to be removed. Code cleanup for unnecessary code is always a good practice.
Refer: http://msdn.microsoft.com/en-us/magazine/ee335722.aspx.
'null' check needs to be performed wherever applicable to avoid the Null Reference Exception at runtime.
Naming conventions to be followed always. Generally for variables/parameters, follow Camel casing and for method names and class names, follow Pascal casing.
Refer: http://msdn.microsoft.com/en-us/library/ms229043.aspx.
Make sure that you are aware of SOLID principles.
Definition from Wikipedia: In computer programming, SOLID (Single responsibility, Open-closed, Liskov substitution, Interface segregation and Dependency inversion) is a mnemonic acronym introduced by Michael Feathers for the "first five principles" identified by Robert C. Martin^[1]^[2] in the early 2000s^[3] that stands for five basic principles of object-oriented programming and design. The principles when applied together intend to make it more likely that a programmer will create a system that is easy to maintain and extend over time.^[3] The principles of SOLID are guidelines that can be applied while working on software to remove code smells by causing the programmer to refactor the software's source code until it is both legible and extensible. It is typically used with test-driven development, and is part of an overall strategy of agile and adaptive programming.

Refer: http://en.wikipedia.org/wiki/SOLID_(object-oriented_design)
Code Reusability: Extract a method if the same piece of code is being used more than once or you expect it to be used in future. Make some generic methods for repetitive task and put them in a related class so that other developers start using them once you intimate them. Develop user controls for common functionality so that they can be reused across the project.
Refer:
- http://msdn.microsoft.com/en-us/library/office/aa140806(v=office.10).aspx
- http://blogs.msdn.com/b/frice/archive/2004/06/11/153709.aspx
Code Consistency: Let's say that an Int32 type is coded as int and String type is coded as string, then they should be coded in that same fashion across the application. But not like sometimes int and sometimes as Int32.
Code Readability: Should be maintained so that other developers understand your code easily.
Refer: http://msdn.microsoft.com/en-IN/library/aa291591(v=vs.100).aspx
Disposing of Unmanaged Resources like File I/O, Network resources, etc. They have to be disposed of once their usage is completed. Use usings block for unmanaged code, if you want to automatically handle the disposing of objects once they are out of scope.
Refer: http://msdn.microsoft.com/en-us/library/498928w2.aspx
Proper implementation of Exception Handling (try/catch and finally blocks) and logging of exceptions.
Refer: http://msdn.microsoft.com/en-us/library/vstudio/ms229005(v=vs.100).aspx
Making sure that methods have less number of lines of code. Not more than 30 to 40 lines.
Timely check-in/check-out of files/pages at source control (like TFS).
Refer: http://www.codeproject.com/Tips/593014/Steps-Check-in-Check-Out-Mechanism-for-TFS-To-avoi
Peer code reviews. Swap your code files/pages with your colleagues to perform internal code reviews.
Unit Testing. Write developer test cases and perform unit testing to make sure that basic level of testing is done before it goes to QA testing.
Refer: https://msdn.microsoft.com/en-us/library/hh694602.aspx
Avoid nested for/foreach loops and nested if conditions as much as possible.
Use anonymous types if code is going to be used only once.
Refer: http://msdn.microsoft.com/en-us/library/vstudio/bb397696.aspx
Try using LINQ queries and Lambda expressions to improve Readability.
Refer: http://msdn.microsoft.com/en-us/library/bb308959.aspx
Proper usage of var, object, and dynamic keywords. They have some similarities due to which most of the developers are confused or don’t know much about them and hence they use them interchangeably, which shouldn't be the case.
Refer: http://blogs.msdn.com/b/csharpfaq/archive/2010/01/25/what-is-the-difference-between-dynamic-and-object-keywords.aspx
Use access specifiers (private, public, protected, internal, protected internal) as per the scope need of a method, a class, or a variable. Let's say if a class is meant to be used only within the assembly, then it is enough to mark the class as internal only.
Refer: http://msdn.microsoft.com/en-us/library/kktasw36.aspx
Use interfaces wherever needed to maintain decoupling. Some design patterns came into existence due to the usage of interfaces.
Refer: http://msdn.microsoft.com/en-IN/library/3b5b8ezk(v=vs.100).aspx
Mark a class as sealed or static or abstract as per its usage and your need.
Refer: http://msdn.microsoft.com/en-us/library/ms173150(v=vs.100).aspx
Use a Stringbuilder instead of string if multiple concatenations are required, to save heap memory.
Check whether any unreachable code exists and modify the code if it exists.
Write comments on top of all methods to describe their usage and expected input types and return type information.
Use a tool like Silverlight Spy to check and manipulate rendered XAML in Runtime of a Silverlight application to improve productivity. This saves lot of back & forth time between Design & Run views of the XAML.
Use fiddler tool to check the HTTP/network traffic and bandwidth information to trace the performance of web application and services.
Use WCFTestClient.exe tool if you want to verify the service methods out of the Visual Studio or by attaching its process to Visual Studio for debugging purposes.
Use constants and readonly wherever applicable.
Refer:
- http://msdn.microsoft.com/en-us/library/acdd6hb7(v=vs.100).aspx
- http://msdn.microsoft.com/en-us/library/e6w8fe1b(v=vs.100).aspx
Avoid type casting and type conversions as much as possible; because it is a performance penalty.
Refer: http://msdn.microsoft.com/en-us/library/ms173105.aspx
Override ToString (from Object class) method for the types which you want to provide with custom information.
Refer: http://msdn.microsoft.com/en-us/library/ms173154(v=vs.100).aspx
Avoid straightaway copy/pasting of code from other sources. It is always recommended to hand write the code even though if you are referring to the code from some sources. By this, you will get good practice of writing the code yourself and also you will understand the proper usage of that code; finally you will never forget it.
Always make it a practice to read books/articles, upgrade and follow the Best Practices and Guidelines by industry experts like Microsoft experts and well-known authors like Martin Fowler, Kent Beck, Jeffrey Ritcher, Ward Cunningham, Scott Hanselman, Scott Guthrie, Donald E Knuth.
Verify whether your code have any memory leakages. If yes, make sure that they have been fixed.
Refer: http://blogs.msdn.com/b/davidklinems/archive/2005/11/16/493580.aspx
Try attending technical seminars by experts to be in touch with the latest software trends and technologies and best practices.
Understand thoroughly the OOPs concepts and try implementing it in your code.
Get to know about your project design and architecture to better understand the flow of your applicationas a whole.
Take necessary steps to block and avoid any cross scripting attacks, SQL injection, and other security holes.
Always encrypt (by using good encryption algorithms) secret/sensitive information like passwords while saving to database and connection strings stored in web.config file(s) to avoid manipulation by unauthorized users.
Avoid using default keyword for the known types (primitive types) like int, decimal, bool, etc. Most of the times, it should be used in case of Generic types (T) as we may not be sure whether the type is a value type or reference type.
Refer: http://msdn.microsoft.com/en-us/library/xwth0h0d(v=vs.100).aspx
Usage of 'out' and 'ref' keywords be avoided as recommended by Microsoft (in the Code analysis Rules and guidelines). These keywords are used to pass parameters by reference. Note that 'ref' parameter should be initialized in the calling method before passing to the called method but for 'out' parameter this is not mandatory.