Inappropriate coding practices - Cyclomatic complexity

Need

Improvement of coding practices to reduce cyclomatic complexity

Context

• Usage of Python 3 for developing applications and scripts
• Usage of request for making HTTP requests

Description

Non compliant code

def complex_function(request, model_name, id, action):
    if model_name == "User":
        if action == "create":
            if request.method == "POST":
                # code to create user
                pass
            elif request.method == "GET":
                # code to show form
                pass
        elif action == "delete":
            if request.user.is_admin:
                # code to delete user
                pass
            else:
                # code to show error
                pass
    elif model_name == "Post":
        if action == "create":
            if request.method == "POST":
                # code to create post
                pass
            elif request.method == "GET":
                # code to show form
                pass
        elif action == "delete":
            if request.user.is_admin:
                # code to delete post
                pass
            else:
                # code to show error
                pass

The above code is a function in a Django backend application that handles different actions (like create, delete) for different models (like User, Post). The function takes four parameters: the request object, the name of the model, the id of the model instance, and the action to be performed.

The function suffers from high McCabe cyclomatic complexity. Cyclomatic complexity is a software metric used to indicate the complexity of a program. It is a quantitative measure of the number of linearly independent paths through a program's source code. The cyclomatic complexity of the above function is high because it has many independent paths - one for each combination of model name, action, request method, and user type.

High cyclomatic complexity increases the likelihood of bugs and makes the code harder to understand, maintain, and test. This function, for example, would require a large number of tests to cover all possible paths, many of which involve nested conditionals, which are harder to reason about.

Additionally, the high complexity of this function can lead to the introduction of new security vulnerabilities, as it is more difficult to ensure that all paths through the function are secure. It also makes it more difficult to detect and fix existing vulnerabilities.

Steps

• Refactor the complex_function by breaking it down into smaller, more manageable functions. Each function should have a single responsibility.
• Create separate functions for each model_name (User, Post) and action (create, delete).
• Create separate functions for handling different request methods (POST, GET).
• Use a design pattern such as Command or Strategy to encapsulate each action as an object. This can simplify the code and reduce its complexity.
• Consider using a routing library or framework that can handle the routing logic, reducing the complexity of your code.
• Use polymorphism to handle different types of models (User, Post) and actions (create, delete). This can help to reduce the number of conditional statements and thus reduce complexity.

Compliant code

class ActionHandler:
    def post(self):
        pass

    def get(self):
        pass

class CreateUserHandler(ActionHandler):
    def post(self):
        # code to create user
        pass

    def get(self):
        # code to show form
        pass

class DeleteUserHandler(ActionHandler):
    def post(self):
        if request.user.is_admin:
            # code to delete user
            pass
        else:
            # code to show error
            pass

class CreatePostHandler(ActionHandler):
    def post(self):
        # code to create post
        pass

    def get(self):
        # code to show form
        pass

class DeletePostHandler(ActionHandler):
    def post(self):
        if request.user.is_admin:
            # code to delete post
            pass
        else:
            # code to show error
            pass

def handle_request(request, model_name, id, action):
    handlers = {
        'User': {
            'create': CreateUserHandler(),
            'delete': DeleteUserHandler()
        },
        'Post': {
            'create': CreatePostHandler(),
            'delete': DeletePostHandler()
        }
    }

    handler = handlers[model_name][action]
    if request.method == 'POST':
        handler.post()
    elif request.method == 'GET':
        handler.get()

The code has been refactored to reduce its cyclomatic complexity. The original function, complex_function, has been broken down into smaller, more manageable functions. Each function now has a single responsibility, which makes the code easier to understand and maintain.

The ActionHandler class is a base class for handling different actions. It has two methods, post and get, which are overridden in the derived classes.

The CreateUserHandler, DeleteUserHandler, CreatePostHandler, and DeletePostHandler classes are derived from the ActionHandler class. Each of these classes represents a specific action for a specific model. They override the post and get methods of the ActionHandler class to provide the specific functionality for their respective actions.

The handle_request function is a simplified version of the original complex_function. It uses a dictionary, handlers, to map model names and actions to their respective handler objects. It then calls the appropriate method on the handler object based on the request method.

This refactoring reduces the cyclomatic complexity of the code by reducing the number of conditional statements. It also makes the code more modular and easier to maintain.

References

• 145.Inappropriate coding practices - Cyclomatic complexity