Inappropriate coding practices - Cyclomatic complexity

Need

Improvement of code quality and reduction of cyclomatic complexity

Context

• Usage of Scala for building scalable and high-performance applications

Description

Non compliant code

def complexMethod(a: Int, b: Int, c: Int, d: Int, e: Int, f: Int): Int = {
  if (a > b) {
    if (b > c) {
      if (c > d) {
        if (d > e) {
          if (e > f) {
            return a
          } else {
            return f
          }
        } else {
          if (d > f) {
            return d
          } else {
            return f
          }
        }
      } else {
        if (c > e) {
          if (e > f) {
            return c
          } else {
            return f
          }
        } else {
          if (c > f) {
            return c
          } else {
            return f
          }
        }
      }
    } else {
      if (b > d) {
        if (d > e) {
          if (e > f) {
            return b
          } else {
            return f
          }
        } else {
          if (b > f) {
            return b
          } else {
            return f
          }
        }
      } else {
        if (b > e) {
          if (e > f) {
            return b
          } else {
            return f
          }
        } else {
          if (b > f) {
            return b
          } else {
            return f
          }
        }
      }
    }
  } else {
    if (a > c) {
      if (c > d) {
        if (d > e) {
          if (e > f) {
            return a
          } else {
            return f
          }
        } else {
          if (a > f) {
            return a
          } else {
            return f
          }
        }
      } else {
        if (a > d) {
          if (d > f) {
            return a
          } else {
            return f
          }
        } else {
          if (a > f) {
            return a
          } else {
            return f
          }
        }
      }
    } else {
      if (a > e) {
        if (e > f) {
          return a
        } else {
          return f
        }
      } else {
        if (a > f) {
          return a
        } else {
          return f
        }
      }
    }
  }
}

The above Scala code represents a method with high cyclomatic complexity. The method, complexMethod, takes six integer parameters and returns an integer. The purpose of the method is to find the maximum value among the six input parameters.

The cyclomatic complexity of a method is calculated based on the number of linearly independent paths through the method's source code. In this case, the cyclomatic complexity is very high due to the numerous nested if statements. Each if statement introduces a new path through the code, increasing the complexity.

This high complexity makes the code harder to understand, maintain, and test. It also increases the likelihood of introducing new bugs or security vulnerabilities, as it's challenging to predict the behavior of the code in all possible scenarios.

Furthermore, if the code needs to be updated or modified, the high complexity could make it difficult to implement changes without introducing new issues. This is because the developer would need to understand all the different paths through the code and how they interact, which can be difficult with such complex code.

In summary, the high cyclomatic complexity of this code makes it vulnerable to bugs and security issues, and hinders maintainability and understandability.

Steps

• Refactor the complexMethod to reduce cyclomatic complexity.
• Break down the method into smaller, more manageable functions.
• Use conditional statements and loops instead of nested if-else statements.
• Apply the Single Responsibility Principle to ensure each function has a single purpose.
• Consider using pattern matching or polymorphism to simplify the code logic.
• Apply code review and pair programming to identify and refactor complex code sections.
• Use automated tools to analyze and detect high cyclomatic complexity.
• Provide proper documentation and comments to improve code readability and maintainability.

Compliant code

def maxOfTwo(a: Int, b: Int): Int = {
  if (a > b) a else b
}

def maxOfThree(a: Int, b: Int, c: Int): Int = {
  maxOfTwo(maxOfTwo(a, b), c)
}

def complexMethod(a: Int, b: Int, c: Int, d: Int, e: Int, f: Int): Int = {
  maxOfThree(maxOfThree(a, b, c), maxOfThree(d, e, f))
}

The original complexMethod had a high cyclomatic complexity due to the nested if-else statements. This made the code difficult to understand, maintain, and prone to security vulnerabilities.

The refactored code reduces the cyclomatic complexity by breaking down the problem into smaller, more manageable functions.

• maxOfTwo(a: Int, b: Int): Int is a helper function that takes two integers and returns the maximum of the two.
• maxOfThree(a: Int, b: Int, c: Int): Int is another helper function that uses maxOfTwo to find the maximum of three integers.
• complexMethod(a: Int, b: Int, c: Int, d: Int, e: Int, f: Int): Int is the main function that now uses maxOfThree to find the maximum of six integers.

This refactoring simplifies the code logic, making it easier to understand and maintain. It also reduces the risk of introducing new security vulnerabilities.

The Single Responsibility Principle is applied to ensure each function has a single purpose. This makes the code easier to test and debug.

Automated tools can be used to analyze and detect high cyclomatic complexity in the code. Code review and pair programming can also help identify and refactor complex code sections.

Proper documentation and comments are provided to improve code readability and maintainability.

References

• 145.Inappropriate coding practices - Cyclomatic complexity