Refactor this method to reduce its Cognitive Complexity

Question

I'm having a problem with refactoring this part of the code. How to reduce the cognitive complexity of this code

if (fieldseparator != null && !fieldseparator.isEmpty()) {
        if (fieldseparator.equals("\\t")) {
            delim = '\t';
        } else {
            // handling unicode separator
            Integer i = Ints.tryParse(fieldseparator);
            if (i != null) {
                fieldseparator = String.valueOf(Character.toChars(i));
                delim = fieldseparator.charAt(0);
            } else {
                delim = fieldseparator.charAt(0);
            }
        }
    }

Answer 1

public static Character getDelim(String fieldseparator, Character defaultDelim) {
    if (isNotEmpty(fieldseparator)) {
        return fieldseparator.equals("\\t") ? '\t' : getFromUnicode(fieldseparator);
    }
    return defaultDelim;
}

private static Character getFromUnicode(String fieldseparator) {
    Integer i = Ints.tryParse(fieldseparator);
    return i != null ? String.valueOf(Character.toChars(i)).charAt(0) : fieldseparator.charAt(0);
}

private static boolean isNotEmpty(String fieldseparator) {
    return fieldseparator != null && !fieldseparator.isEmpty();
}

Answer 2

For a general explanation about cognitive complexity and how it is calculated, please have a look at this answer: https://stackoverflow.com/a/62867219/7730554.

First of all, I want to state that I am not trying to come up with a better solution than already provided by jahra (which is great). I rather want to share how cognitive complexity can be reduced by taking safe baby steps applying well-known refactoring techniques. The final result is of course very similar to the one already shown in the existing answer.

Concerning this specific example you can pretty easy achieve a cognitive complexity reduction by applying simple refactorings such as Replace Nested Condition with Guard Clause, Consolidate Duplicate Conditional Fragments and Extract Method. I applied these as shown in the following code listings as emphasized in the code comments.

Note: nowadays most IDEs already provide these refactorings as automated refactorings out of the box, in my case I used IntelliJ (for Java) which provides all of the mentioned refactorings so I did not have to these manually. Other technology stacks provide similar features, e.g. Rider or ReSharper for .Net, PHPStorm for PHP, etc.

Let's first calculate the cognitive complexity of the original code (see comments to follow the calculation of each relevant statement):

// Total cognitive complexity: 9
public char findDelimiter_original(String fieldseparator)
{
    char delim = '\0';
    if (fieldseparator != null // +1 for the if condition
            && !fieldseparator.isEmpty()) { // +1 for the binary operator
        if (fieldseparator.equals("\\t")) { // +2 for the nested if (nesting = 1)
            delim = '\t';
        } else { // +1 for else
            // handling unicode separator
            Integer i = Ints.tryParse(fieldseparator);
            if (i != null) { // +3 for the double nested if (nesting = 2)
                fieldseparator = String.valueOf(Character.toChars(i));
                delim = fieldseparator.charAt(0);
            } else { // +1 for the else
                delim = fieldseparator.charAt(0);
            }
        }
    }
    return delim;
}

So the original code leaves us with a cognitive complexity of 9.

In the next step I applied the Replace Nested Condition with Guard Clause and the Consolidate Duplicate Conditional Fragments refactorings.

Note: using IntelliJ Replace Nested Condition with Guard Clause can be easily achieved via "Invert if condition" while Consolidate Duplicate Conditional Fragments can be achieved via "Extract common part from if" from the IntelliJ context menu.

// Total cognitive complexity: 4
public char findDelimiter_guardClauses(String fieldseparator)
{
    // Applied "guard clause" refactoring by inverting if
    if (fieldseparator == null // +1 for the if condition
            || fieldseparator.isEmpty()) { // +1 for the binary operator
        return '\0';
    }

    // Applied "guard clause" refactoring by inverting if
    if (fieldseparator.equals("\\t")) { // +1 the if condition
        return '\t';
    }

    // Applied "consolidate duplicate conditional fragments" refactoring
    // handling unicode separator
    Integer i = Ints.tryParse(fieldseparator);
    if (i != null) { // +1
        fieldseparator = String.valueOf(Character.toChars(i));
    }
    return fieldseparator.charAt(0);
}

This now leaves us with a reduced cognitive complexity of 4 which is already great, but we can still do better, especially in terms of readability.

In the last step I applied the Extract Method refactoring which is also available in IntelliJ as automated refactoring.

// Total cognitive complexity: 2
public char findDelimiter_extractMethod(String fieldseparator)
{
    // Applied "extract method" refactoring
    if (hasNoSeparator(fieldseparator)) { // +1 for the if condition
        return '\0';
    }

    // Applied "extract method" refactoring
    if (fieldSeparatorIsTab(fieldseparator)) { // +1 for the if condition
        return '\t';
    }

    // Applied "extract method" refactoring
    return handleUnicodeString(fieldseparator).charAt(0);
}

// Total complexity: 1
private String handleUnicodeString(String fieldseparator) {
    Integer i = Ints.tryParse(fieldseparator);
    if (i != null) { // +1 for the if condition
        fieldseparator = String.valueOf(Character.toChars(i));
    }
    return fieldseparator;
}

// Total cognitive complexity: 0
private boolean fieldSeparatorIsTab(String fieldseparator) {
    return fieldseparator.equals("\\t");
}

// Total cognitive complexity: 1
private boolean hasNoSeparator(String fieldseparator) {
    return fieldseparator == null || fieldseparator.isEmpty(); // +1 for the binary operator
}

The overall code (including the extracted private methods) still has an overall cognitive complexity of 4, while the main method now only has a cognitive complexity of 2.

Finally we can replace the last if condition with the ternery operator (which is also provided by IntelliJ via "replace if else with ?".

// Total cognitive complexity: 2
public char findDelimiter_extractMethod(String fieldseparator)
{
    // Applied "extract method" refactoring
    if (hasNoSeparator(fieldseparator)) { // +1 for the if condition
        return '\0';
    }

    // Introduced ternary operator
    // +1 for the ternary operator
    return fieldSeparatorIsTab(fieldseparator) ? '\t' : handleUnicodeString(fieldseparator).charAt(0);
}