Skip to main content

Inappropriate coding practices - Cyclomatic complexity

Need​

Improvement of coding practices to reduce cyclomatic complexity

Context​

  • Usage of Dart for building cross-platform mobile, web, and desktop applications
  • Usage of Shelf for managing and organizing data in a Node.js application

Description​

Non compliant code​

import 'package:shelf/shelf.dart' as shelf;

shelf.Response handleRequest(shelf.Request request) {
  var method = request.method;
  var url = request.requestedUri.path;

  if (method == 'GET') {
    if (url == '/home') {
      return shelf.Response.ok('Home Page');
    } else if (url == '/about') {
      return shelf.Response.ok('About Page');
    } else if (url == '/contact') {
      return shelf.Response.ok('Contact Page');
    } else if (url == '/help') {
      return shelf.Response.ok('Help Page');
    } else if (url == '/privacy') {
      return shelf.Response.ok('Privacy Page');
    } else if (url == '/terms') {
      return shelf.Response.ok('Terms Page');
    } else {
      return shelf.Response.notFound('Page Not Found');
    }
  } else if (method == 'POST') {
    if (url == '/login') {
      return shelf.Response.ok('Login Page');
    } else if (url == '/signup') {
      return shelf.Response.ok('Signup Page');
    } else if (url == '/logout') {
      return shelf.Response.ok('Logout Page');
    } else {
      return shelf.Response.notFound('Page Not Found');
    }
  } else {
    return shelf.Response.forbidden('Method Not Allowed');
  }
}

This code has a high cyclomatic complexity due to the nested if and else if statements. Each if or else if statement adds a path through the code, increasing the cyclomatic complexity. This makes the code harder to understand, maintain, and test.

In addition, the code's complexity can lead to the introduction of new security vulnerabilities. For example, if a new endpoint is added, it might be easy to forget to add it to the list of allowed endpoints. This could lead to unauthorized access to the endpoint.

Moreover, the high complexity of the code makes it harder to detect and fix existing vulnerabilities. For example, if a vulnerability is found in the way the application handles requests, it will be harder to locate the vulnerable code due to the high number of paths through the code.

To mitigate this, the code can be refactored to reduce the number of conditional statements, thus reducing the cyclomatic complexity. This can be achieved by using a routing library or creating a map of routes and their corresponding handlers.

Steps​

  • Identify the parts of the code that are causing high cyclomatic complexity. In this case, it's the nested if-else statements.
  • Refactor the code to reduce the number of decision points. This can be achieved by using a switch-case statement or a map/dictionary to handle different cases.
  • For the 'GET' and 'POST' methods, create a map/dictionary where the keys are the URLs and the values are the corresponding responses. This will eliminate the need for multiple if-else statements.
  • For the method check, use a switch-case statement instead of if-else. The cases would be 'GET', 'POST', and a default case to handle other methods.
  • By doing these changes, the cyclomatic complexity will be reduced, making the code easier to understand and maintain.

Compliant code​

import 'package:shelf/shelf.dart' as shelf;

// Define a map for GET requests
final Map<String, String> getResponses = {
  '/home': 'Home Page',
  '/about': 'About Page',
  '/contact': 'Contact Page',
  '/help': 'Help Page',
  '/privacy': 'Privacy Page',
  '/terms': 'Terms Page',
};

// Define a map for POST requests
final Map<String, String> postResponses = {
  '/login': 'Login Page',
  '/signup': 'Signup Page',
  '/logout': 'Logout Page',
};

shelf.Response handleRequest(shelf.Request request) {
  var method = request.method;
  var url = request.requestedUri.path;

  switch (method) {
    case 'GET':
      return getResponses.containsKey(url)
          ? shelf.Response.ok(getResponses[url])
          : shelf.Response.notFound('Page Not Found');
    case 'POST':
      return postResponses.containsKey(url)
          ? shelf.Response.ok(postResponses[url])
          : shelf.Response.notFound('Page Not Found');
    default:
      return shelf.Response.forbidden('Method Not Allowed');
  }
}

The refactored code reduces the cyclomatic complexity by eliminating the nested if-else statements and replacing them with a switch-case statement and map lookups.

The handleRequest function now uses a switch-case statement to handle different HTTP methods. For 'GET' and 'POST' methods, it looks up the requested URL in the corresponding map (getResponses or postResponses). If the URL is found in the map, it returns a response with the corresponding value from the map. If the URL is not found, it returns a 'Page Not Found' response.

For any other HTTP methods, the function returns a 'Method Not Allowed' response.

This refactoring makes the code easier to understand and maintain, and reduces the risk of introducing new security vulnerabilities.

References​

Last updated on