CODEWITHSUNDEEP
coding-stylecode-cleanup
uncle bob
uncle bob

Reputation: 680

How to reduce the if-else depth?

I have this example code

public static ActionProcessable getActionProcessor(TaskType currentTaskType, UserAction userAction){
    String actionKey;
    if(userAction != null){
        if(currentTaskType != null){
            actionKey = buildKey(currentTaskType, userAction);
            if(dossierActions.containsKey(actionKey)){
                return dossierActions.get(actionKey);
            }
        }

        actionKey = buildKey(anyTaskType(), userAction);
        if(dossierActions.containsKey(actionKey)){
            return dossierActions.get(actionKey);
        }
    }

    return new NullActionProcessor();
}

In this logic i have a map to store the ActionProcessable by combined-key TaskType and UserAction. This method will return ActionProcessable with input taskType and action. TaskType can be null so in that case we only need to get by userAction.

When i check this code by sonar, it say the third if is "Nested if-else depth is 2 (max allowed is 1)"

But i don't know how to make it better. Does anyone suggest me something?

Upvotes: 2

Views: 1105

Answers (1)

Yeldar Kurmangaliyev
Yeldar Kurmangaliyev

Reputation: 34189

You can move "if containsKey" part out of condition to remove code duplication:

public static ActionProcessable getActionProcessor(TaskType currentTaskType, UserAction userAction){
    if (userAction != null) {
        String actionKey = currentTaskType != null
            ? buildKey(currentTaskType, userAction)
            : buildKey(anyTaskType(), userAction);

        if (dossierActions.containsKey(actionKey)){
            return dossierActions.get(actionKey);
        }
    }

    return new NullActionProcessor();
}

Now, intent of the code looks more clear (at least, for me).

You can also make the first condition short-circuit and\or use ternary if for containsKey, it will remove even more ifs, but can make code more complex for some people. 

public static ActionProcessable getActionProcessor(TaskType currentTaskType, UserAction userAction){
    if (userAction == null) { 
        return new NullActionProcessor();
    }

    String actionKey = currentTaskType != null
        ? buildKey(currentTaskType, userAction)
        : buildKey(anyTaskType(), userAction);

    return dossierActions.containsKey(actionKey)
        ? dossierActions.get(actionKey);
        : new NullActionProcessor();
}

Choose the one you like, they are technically similar.

Since you haven't specified specific programming language, one more thing to say: your code is a good example of use case of null-coalsecing operator. Sadly, AFAIK, there is none in Java. In C#, the code could look like this:

public static ActionProcessable GetActionProcessor(TaskType currentTaskType, UserAction userAction) {
    if (userAction == null) { 
        return new NullActionProcessor();
    }

    var actionKey = BuildKey(currentTaskType ?? anyTaskType(), userAction);
    return dossierActions[actionKey] ?? new NullActionProcessor();
}

Upvotes: 1

Related Questions