Skip to main content

Inappropriate coding practices - Cyclomatic complexity

Need​

Improvement of coding practices to reduce cyclomatic complexity

Context​

  • Usage of Ruby for building dynamic and object-oriented applications
  • Usage of Ruby for building dynamic and object-oriented applications

Description​

Non compliant code​

def complex_method(user)
  if user
    if user.active?
      if user.age > 18
        if user.has_permission?
          if user.admin?
            "Admin"
          else
            "User with permission"
          end
        else
          "User without permission"
        end
      else
        "Minor"
      end
    else
      "Inactive user"
    end
  else
    "No user"
  end
end

The above code is a method (complex_method) in a Ruby on Rails application that checks the status of a user. The method has a high cyclomatic complexity due to the numerous nested if conditions.

Here's a breakdown of the conditions:

  1. It first checks if the user exists.
  2. If the user exists, it checks if the user is active.
  3. If the user is active, it checks if the user is above 18 years old.
  4. If the user is above 18, it checks if the user has permissions.
  5. If the user has permissions, it checks if the user is an admin.

This method has a cyclomatic complexity of 6, which is the number of independent paths through the code. This high complexity makes the code difficult to understand and maintain. It also promotes the appearance of new security vulnerabilities and hinders their detection and solution.

For instance, if a new condition needs to be added or an existing condition needs to be modified, there is a high risk of introducing errors due to the intricate nesting of conditions. Furthermore, it's challenging to write unit tests that cover all possible paths through the code, which increases the risk of undetected bugs and vulnerabilities.

Steps​

  • Reduce the cyclomatic complexity by refactoring the code.
  • Use guard clauses to handle the conditions and return early.
  • Simplify the logic by using conditional statements instead of nested if statements.
  • Consider using a switch statement or a lookup table for better readability.
  • Extract complex conditions into separate methods or helper functions.
  • Apply the Single Responsibility Principle to split the method into smaller, more focused methods.
  • Consider using a state machine or a design pattern to handle the different user states.
  • Use meaningful variable and method names to improve code readability.
  • Consider using a linter or static code analysis tool to identify and refactor high complexity code.

Compliant code​

def user_status(user)
  return "No user" unless user
  return "Inactive user" unless user.active?
  return "Minor" unless user.age > 18
  return "User without permission" unless user.has_permission?

  user.admin? ? "Admin" : "User with permission"
end

The refactored code reduces the cyclomatic complexity by using guard clauses. Guard clauses are conditional statements that allow us to exit a method early when a certain condition is met. This reduces the number of nested if statements, making the code easier to read and understand.

In the refactored code, we first check if the user exists. If not, we return "No user" and exit the method. If the user does exist, we then check if they are active. If they are not active, we return "Inactive user" and exit the method. We continue this pattern for the remaining conditions.

Finally, if the user has passed all the previous checks, we use a ternary operator to check if they are an admin. If they are, we return "Admin". If they are not, we return "User with permission".

This refactoring significantly reduces the cyclomatic complexity of the method, making it easier to maintain and less prone to bugs and security vulnerabilities. It also improves the readability of the code, making it easier for other developers to understand.

References​

Last updated on