Skip to main content

TnT Code Review Implementation Procedure

Version: 1, effective date: 01-Dec-2021

Aleksandr Illarionov

Contents​

1 DOCUMENT OBJECTIVES​

The purpose of this document is to provide instruction on implementing T&T Code review standard for an application.

2 CODE PREPARATION​

The Web Developer will use Entity relationship database diagram (containing standard entity relationships. i.e. information about tables, views, columns, data types, constraints, triggers and indexes) and standard UML diagrams for detailed specifications (see 10.073E Sample Class Diagram).

During development, the Web Developer will adopt standard coding guidelines (see C# Coding Guidelines (10.073F) ) and will produce the list of stored procedures, user defined functions, jobs etc. with a short description of their role. All classes/functions/stored procedures/user defined functions etc. are commented using a standard, portable code comments format (see Database Coding Guidelines (10.073G) ).

The Web Developer will follow JTI 10.56 Secure Web Development Guideline to ensure proper security measures are implemented in the code

2.1 Source code annotation​

The author should annotate source code before reviewing which files to look at first and the reason behind each code modification. To ease the code review process and provide more depth in context.

2.2 What should be done before review?​

2.2.1 .NET application​

  • All warnings in a build must be resolved.

  • Use unit tests if possible.

  • All connection strings must be set in configuration file “Web.config” or “App.config” (or appsetting.json for .NET Core)

  • Use transformation of “Web.config” (or appsetting.json for .NET Core) for all Configurations.

  • Each external library must be included in the source code and mentioned in the .csproj file.

  • Edit csproj and ensure you have the below version nodes (used for automatic versioning via GitVersion (required)

<PropertyGroup>
  <AssemblyVersion>1.0.0.0</AssemblyVersion>
  <FileVersion>1.0.0.0</FileVersion>
  ...
</PropertyGroup>

2.2.2 Database​

  • Each application must contain a db-project for each database that is used.

  • All database projects must contain an initial script for deployment in a new environment.

  • Database project has to contain a db role with permissions enough for running application (ex. application)

  • All users, logins must be excluded from db project. (db roles only)

2.2.3 Prepare documentations​

Authors should annotate code before the review occurs because annotations guide the reviewer through the changes, showing which files to look at first and defending the reason behind each code modification. Annotations should be directed at other reviewers to ease the process and provide more depth in context. 

Following documents should be created or updated:

To clarify how to define software requirements, please see: Guiding Template for Software Requirements (10.073D)

3 CODE REVIEW PROCESS​

After a developer completes coding, it raises a pull request. Another developer or more developers (reviewers) review the code in the pull request.

Reviewers check code for potential bugs, code styling conventions, matching functional requirements and code. Found issues registered and discussed with author. Author fixes issues.

When all issues are fixed, pull-request merges.

Use checklist

When doing code reviews or reporting bugs, frequently made errors must be collected into a checklist. Each code review must be checked by this list.

Establish a process for fixing defects found

Recommended to log found bugs during code review, discuss them with author and approve changes in code.

Stablish automated process

In addition to code review, use static code analyzers. It allows detecting potential problems and vulnerabilities automatically.

  • Assessing the code quality of the entire project is done through a static code analysis tool. Use the tools (based on technology) such as SonarQube, SonarCloud or another linter.
  • Use plug-ins such as Resharper, which suggests the best practices in Visual studio.

3.1 What should be reviewed?​

3.1.1 Code formatting​

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

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

  • Ensure that proper naming conventions have been followed.

  • 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 can be opened while modifying code, so always write code keeping in view a 14 inch monitor.

3.1.2 Architecture​

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).

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

  2. Design patterns: Use appropriate design pattern, after completely understanding the problem and context.

3.1.3 Coding best practices​

  • No hard coding, use constants/configuration values.

  • Group similar values under an enumeration (enum).

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

  • Avoid multiple if/else blocks.

  • Use framework features wherever possible instead of writing custom code.

Your code has to follow with best practices of that technology which you are using.

Please use the following documents to check C# projects: MMDC Coding Guidelines

And this for projects of database: Database Coding Guidelines (10.073G)

3.2 Non-Functional requirements​

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 an appropriate name for variables, functions and classes. If you are taking more time to understand the code, either code needs refactoring or at least comments ought 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.

  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.

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.

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

Extensibility – Easy to add enhancements with minimal changes to the existing code. A component should be easily replaceable with a better version.

Security - Authentication, authorization, input data validation against security threats such as SQL injections and Cross Site Scripting (XSS), encrypting the sensitive data (passwords, secret keys, credentials details, etc.)

Checking of following JTI policies:

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.

Scalability – Consider if it supports an extensive user base/data. Can this be deployed into web farms?

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

3.3 Object-Oriented Analysis and Design (OOAD) Principles​

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

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

  • 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.

  • 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.

  • 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 example, if the “Single Responsibility Principle” is followed, Reusability and Testability will automatically increase.

In a few cases, one requirement may contradict another requirement. Too many checks and logging at multiple layers (UI, Middle tier, Database) would decrease the performance of an application.

4 Configure automated testing​

Configure unit, integration and automated UI testing for your pipelines.

4.1 Revision History​

VersionEffective datePurpose of changeAuthor
101-Dec-2021First version of the documentAleksandr Illarionov

5 References​

ANY QUESTIONS?

ASK TEAM