CODEWITHSUNDEEP
javacyclomatic-complexity
Robert Lu
Robert Lu

Reputation: 473

Measuring Cyclomatic Complexity

The code below takes in an array of package names and dependencies in the format {Package1: Dependency, Package2: Dependency} and puts them in a hashmap to be returned. According to my interviewer, the method has a cyclomatic complexity of 12, which was is too high to be acceptable. However, I have ran metrics on the code which reported that the complexity is actually 2.

Can someone tell me why the complexity would be so high and how to simplify it for a lower cyclomatic complexity? I am at a loss here.

public static HashMap<String, String> parseDependencies(String[] args) {
    HashMap<String, String> pairs = new HashMap<String, String>();
    for (int i = 0; i < args.length; i++) {
        if (!args[i].contains(": ") || args[i].startsWith(": ")) {
            logger.log(Level.WARNING, ENTRY + args[i] + FORMATTING);
            continue;
        }
        String[] entry = args[i].split(": ");
        if (entry.length < 2) {
            pairs.put(entry[0], null);
        } else {
            pairs.put(entry[0], entry[1]);
        }
    }
    return pairs;
}

Upvotes: 0

Views: 359

Answers (2)

janos
janos

Reputation: 124648

Different tools calculate cyclomatic complexity slightly differently. I don't know how the interview arrived at 12. Or how you arrived at 2. Roughly speaking, cyclomatic complexity counts the number of conditions, loops, and control flow statements. You have one loop, 3 conditions, one continue, else, return, that gives 7, which looks about right.

The implementation doesn't strike me as "complex", but there are a number of issues with it:

  • Return type should be the interface, Map<String, String> instead of HashMap
  • Should use a for-each loop instead of a counting loop
  • The condition on entry.length can be eliminated if you add a second parameter -1 to split. That way, due to the earlier condition on args[i].contains(": ") and the continue that follows, entry.length will be guaranteed to be at least 2.
  • The repetition of : is not great.

I would worry more about these things than cyclomatic complexity in this particular example.

Lastly, a common technique to reduce cyclomatic complexity is to extract code to helper methods. For example, complex boolean conditions, or even an entire loop body.

Upvotes: 1

Stephen C
Stephen C

Reputation: 718698

First of all, it is surprising that an interviewer is relying on Cyclomatic Complexity to measure code written in an interview. I don't think I'd want to work in an "shop" that did that kind of thing routinely.

However, your code could be made simpler from a CC perspective:

public static HashMap<String, String> parseDependencies(String[] args) {
    HashMap<String, String> pairs = new HashMap<String, String>();
    for (int i = 0; i < args.length; i++) {
        String[] entry = args[i].split(": ");
        if (entry.length != 2 || entry[0].isEmpty()) {
           logger.log(Level.WARNING, ENTRY + args[i] + FORMATTING);
        } else if (entry[1].isEmpty()) {
            pairs.put(entry[0], null);
        } else {
            pairs.put(entry[0], entry[1]);
        }
    }
    return pairs;
}

The logic and control flow paths have been simplified and that should reduce the CC measure.

(I also took the opportunity to correct some apparent mistakes.)

Upvotes: 0

Related Questions