Skip to main content

Inappropriate coding practices - Cyclomatic complexity

Need​

Improvement of coding practices to reduce cyclomatic complexity

Context​

  • Usage of Java for building cross-platform applications
  • Usage of javax.servlet for building Java web applications with servlets
  • Usage of javax.servlet.http for handling HTTP requests and responses in Java Servlets

Description​

Non compliant code​

import javax.servlet.*;
import javax.servlet.http.*;

public class ComplexServlet extends HttpServlet {
    protected void doPost(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {
        String action = request.getParameter("action");
        String item = request.getParameter("item");
        String user = request.getParameter("user");
        String password = request.getParameter("password");

        if (action.equals("add")) {
            if (item.equals("book")) {
                if (user.equals("admin")) {
                    if (password.equals("admin")) {
                        // Add book
                    } else {
                        // Error
                    }
                } else {
                    // Error
                }
            } else if (item.equals("cd")) {
                if (user.equals("admin")) {
                    if (password.equals("admin")) {
                        // Add cd
                    } else {
                        // Error
                    }
                } else {
                    // Error
                }
            } else {
                // Error
            }
        } else if (action.equals("delete")) {
            // Similar nested if-else structure for delete
        } else {
            // Error
        }
    }
}

The above code is a Java Servlet that handles HTTP POST requests. The Servlet is designed to perform different actions such as "add" or "delete" based on the parameters received in the request.

The problem with this code is the high McCabe cyclomatic complexity due to the deeply nested if-else control structures. The cyclomatic complexity of a code section is the number of linearly independent paths within it. In this case, each conditional and each branch increases the complexity.

High complexity makes the code more difficult to understand, test, and maintain. It also increases the chances of introducing new bugs and security vulnerabilities. For instance, the deeply nested structure makes it harder to ensure that all error conditions are properly handled. In this case, if a new action or a new item type is added, the developer must correctly modify multiple places in the code.

The code also violates the principle of separation of concerns because it mixes the logic of handling different actions and different item types together. This makes it harder to modify one part of the logic without affecting the others. The code could be refactored to reduce the cyclomatic complexity and improve its maintainability.

Steps​

  • Reduce the cyclomatic complexity of the code
  • Refactor the code to use a more modular and structured approach
  • Implement proper error handling and validation
  • Use appropriate design patterns to simplify the code logic
  • Separate the business logic from the presentation logic
  • Implement a more scalable and maintainable solution

Compliant code​

import javax.servlet.*;
import javax.servlet.http.*;

public class SimplifiedServlet extends HttpServlet {

    protected void doPost(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {
        String action = request.getParameter("action");
        String item = request.getParameter("item");
        String user = request.getParameter("user");
        String password = request.getParameter("password");

        if (!user.equals("admin") || !password.equals("admin")) {
            // Error
            return;
        }

        switch (action) {
            case "add":
                addItem(item);
                break;
            case "delete":
                deleteItem(item);
                break;
            default:
                // Error
                break;
        }
    }

    private void addItem(String item) {
        switch (item) {
            case "book":
                // Add book
                break;
            case "cd":
                // Add cd
                break;
            default:
                // Error
                break;
        }
    }

    private void deleteItem(String item) {
        // Similar structure for delete
    }
}

The refactored code reduces the cyclomatic complexity by using a more structured approach. The nested if-else statements have been replaced with switch-case statements, which are easier to read and maintain.

The code now first checks if the user is an admin and if the password is correct. If either of these conditions is not met, an error is returned and the function ends. This reduces the number of nested conditions and makes the code easier to understand.

The doPost method now only handles the action parameter. Depending on the action, it calls either the addItem or deleteItem method. These methods handle the item parameter. This separation of concerns makes the code more modular and easier to maintain.

The addItem and deleteItem methods are private helper methods that encapsulate the logic for adding and deleting items. This makes the code more scalable, as additional actions or items can be added with minimal changes to the existing code.

The refactored code also improves error handling. If an invalid action or item is provided, an error is returned. This makes the code more robust and secure.

References​

Last updated on