Skip to main content

Inappropriate coding practices - Cyclomatic complexity

Need​

Improvement of coding practices to reduce cyclomatic complexity

Context​

  • Usage of PHP 7.0 for server-side scripting and web development
  • Usage of the user dependency for managing user-related functionality
  • Usage of the item package for managing and manipulating individual items or objects.
  • Usage of location for retrieving and manipulating location data
  • Usage of date for working with dates and time in JavaScript
  • Usage of status for HTTP status code handling and response management

Description​

Non compliant code​

public function complexFunction($user, $item, $location, $date, $status) {
    if($user->hasPermission('can_view_item')) {
        if($item->isAvailable()) {
            if($location->isOpen()) {
                if($date->isWeekday()) {
                    if($status->isActive()) {
                        return true;
                    } else {
                        return false;
                    }
                } else {
                    return false;
                }
            } else {
                return false;
            }
        } else {
            return false;
        }
    } else {
        return false;
    }
}

This function complexFunction has a high McCabe cyclomatic complexity due to the number of linearly independent paths through the function's source code. Cyclomatic complexity is a quantitative measure of the number of linearly independent paths through a program's source code.

The function checks multiple conditions in a nested manner. This increases the complexity as each condition adds a new path through the code. The function checks if the user has permission to view the item, if the item is available, if the location is open, if the date is a weekday, and if the status is active.

This nested if-else structure makes the code hard to read, understand, and maintain. It also increases the likelihood of introducing new bugs or security vulnerabilities in the future, as well as making it more difficult to detect and resolve existing issues.

The complexity of this function could be reduced by breaking it down into smaller, more manageable functions, each with a single responsibility. This would make the code easier to understand, maintain, and test.

Steps​

  • Reduce the number of nested conditions by returning early. This is also known as the 'guard clause' pattern.
  • Instead of nesting if statements, use logical AND operators to combine conditions.
  • Break down the complex function into smaller, more manageable functions. Each function should have a single responsibility.
  • Consider using a design pattern such as the Strategy pattern to encapsulate each condition into its own class, reducing the complexity of the main function.
  • Use unit tests to ensure that the refactored code behaves as expected.

Compliant code​

public function hasPermission($user) {
    return $user->hasPermission('can_view_item');
}

public function isItemAvailable($item) {
    return $item->isAvailable();
}

public function isLocationOpen($location) {
    return $location->isOpen();
}

public function isDateWeekday($date) {
    return $date->isWeekday();
}

public function isStatusActive($status) {
    return $status->isActive();
}

public function complexFunction($user, $item, $location, $date, $status) {
    if($this->hasPermission($user) && $this->isItemAvailable($item) && $this->isLocationOpen($location) && $this->isDateWeekday($date) && $this->isStatusActive($status)) {
        return true;
    }
    return false;
}

The original function complexFunction had a high cyclomatic complexity due to the nested if statements. This made the function difficult to understand, maintain, and test.

To reduce the cyclomatic complexity, the function was refactored using the 'guard clause' pattern and the single responsibility principle. Each condition was moved into its own function, reducing the complexity of the main function and making the code easier to understand and maintain.

The complexFunction now checks if all conditions are met using logical AND operators instead of nested if statements. If all conditions are met, the function returns true. If any condition is not met, the function returns false.

This refactoring makes the code easier to read and understand, reduces the risk of introducing new vulnerabilities, and makes it easier to test each condition independently.

References​

Last updated on