Inappropriate coding practices - Cyclomatic complexity

Need

Improvement of coding practices to reduce cyclomatic complexity

Context

• Usage of C# for building robust and scalable applications
• No specific library or dependency used

Description

Non compliant code

public class ComplexClass
{
    public int ComplexMethod(int a, int b, int c, int d, int e, int f)
    {
        if (a > b)
        {
            if (c > d)
            {
                if (e > f)
                {
                    return a + c + e;
                }
                else
                {
                    return a + c - f;
                }
            }
            else if (e > f)
            {
                return a - d + e;
            }
            else
            {
                return a - d - f;
            }
        }
        else if (c > d)
        {
            if (e > f)
            {
                return b + c + e;
            }
            else
            {
                return b + c - f;
            }
        }
        else if (e > f)
        {
            return b - d + e;
        }
        else
        {
            return b - d - f;
        }
    }
}

The above code represents a method with high cyclomatic complexity. Cyclomatic complexity is a quantitative measure of the number of linearly independent paths through a program's source code. It is computed using the control flow graph of the program.

In the given code, the ComplexMethod has a high cyclomatic complexity due to the numerous conditional branches (if - else if - else statements). Each condition adds 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 security vulnerabilities and makes it difficult to detect and fix existing ones. For instance, each branch in the code would need to be tested separately, which can be time-consuming and error-prone.

Moreover, the complex conditions could lead to unexpected behaviors if not handled correctly, potentially leading to security vulnerabilities. For example, if a new developer were to modify this code, they might inadvertently introduce a bug due to not fully understanding all the possible paths through the code.

Steps

• Refactor the ComplexMethod into smaller, more manageable methods.
• Reduce the number of nested if-else statements by using switch statements or polymorphism.
• Identify and extract common code blocks to reduce duplication.
• Use meaningful variable and method names to improve code readability.
• Apply the Single Responsibility Principle (SRP) to ensure each method has a single, well-defined purpose.
• Consider using design patterns such as Strategy or State to simplify complex logic.
• Use automated code analysis tools to identify and refactor high complexity code.
• Regularly review and refactor the codebase to maintain a low cyclomatic complexity.

Compliant code

public class SimplifiedClass
{
    public int CalculateSum(int a, int b, int c, int d, int e, int f)
    {
        int result = 0;

        result += CompareAndCalculate(a, b);
        result += CompareAndCalculate(c, d);
        result += CompareAndCalculate(e, f);

        return result;
    }

    private int CompareAndCalculate(int first, int second)
    {
        return first > second ? first : -second;
    }
}

The original code had a high cyclomatic complexity due to the nested if-else statements. This made the code difficult to understand and maintain. The refactored code simplifies the logic by breaking down the complex method into smaller, more manageable methods.

The ComplexMethod is refactored into CalculateSum and CompareAndCalculate methods. The CalculateSum method is responsible for calculating the sum of the results of the comparisons. The CompareAndCalculate method is responsible for comparing two numbers and returning the first number if it's greater than the second, otherwise it returns the negative of the second number.

This refactoring reduces the cyclomatic complexity of the code, making it easier to understand and maintain. It also adheres to the Single Responsibility Principle (SRP) as each method now has a single, well-defined purpose. This makes the code more readable and less prone to errors or security vulnerabilities.

In addition, using meaningful method names like CalculateSum and CompareAndCalculate improves the readability of the code. This is a good coding practice as it makes the code self-explanatory.

Regular code reviews and refactoring should be done to maintain a low cyclomatic complexity. Automated code analysis tools can also be used to identify and refactor high complexity code.

References

• 145.Inappropriate coding practices - Cyclomatic complexity