Cyclomatic complexity = 1 + #if statements

code metricscyclomatic-complexityif statement

I found the following paragraph regarding cyclomatic complexity on Wikipedia:

It can be shown that the cyclomatic complexity of any structured program with only one entrance point and one exit point is equal to the number of decision points (i.e., "if" statements or conditional loops) contained in that program plus one.

That would imply a cyclomatic complexity of 3 for two arbitrary nested if statements:

if (a)
{
    if (b)
    {
        foo();
    }
    else
    {
        bar();
    }
}
else
{
    baz();
}

Since exactly one of the three functions is going to be called, my gut agrees with 3.

However, two arbitrary if statements could also be written in sequence instead of nesting them:

if (a)
{
    foo();
}
else
{
    bar();
}

if (b)
{
    baz();
}
else
{
    qux();
}

Now there are four paths through the code:

foo, baz
foo, qux
bar, baz
bar, qux

Shouldn't the cyclomatic complexity of this fragment hence be 4 instead of 3?

Am I misunderstanding the quoted paragraph?

Best Answer

Cyclomatic complexity is defined as the number of linearly independent paths through the code.

In your second example we have the following paths that runs...

| # |   A   |    B  |  Nodes hit   |
| 1 | true  | true  |  foo() baz() |
| 2 | true  | false |  foo() qux() |
| 3 | false | true  |  bar() baz() |
| 4 | false | false |  bar() qux() |

You are completely correct that the number of execution paths here is 4. And yet the cyclomatic complexity is 3.

The key is in understanding what cyclomatic complexity measures:

Definition:

A linearly independent path is any path through the program that introduces at least one new edge that is not included in any other linearly independent paths.

from http://www.ironiacorp.com/

The 4th path is not linearly independent of the first three paths, as it does not introduce any new nodes / program statements that were not included in the first three paths.

As mentioned on the wikipedia article, the cyclomatic complexity is always less than or equal to the number of theoretical unique control flow paths, and is always greater than or equal to the minimum number of actually achievable execution paths.

(to verify the second statement, imagine if b == a was always true when entering the code block you described).

With current logging frameworks, the question is moot

Current logging frameworks like slf4j or log4j 2 don't require guard statements in most cases. They use a parameterized log statement so that an event can be logged unconditionally, but message formatting only occurs if the event is enabled. Message construction is performed as needed by the logger, rather than pre-emptively by the application.

If you have to use an antique logging library, you can read on to get more background and a way to retrofit the old library with parameterized messages.

Are guard statements really adding complexity?

Consider excluding logging guards statements from the cyclomatic complexity calculation.

It could be argued that, due to their predictable form, conditional logging checks really don't contribute to the complexity of the code.

Inflexible metrics can make an otherwise good programmer turn bad. Be careful!

Assuming that your tools for calculating complexity can't be tailored to that degree, the following approach may offer a work-around.

The need for conditional logging

I assume that your guard statements were introduced because you had code like this:

private static final Logger log = Logger.getLogger(MyClass.class);

Connection connect(Widget w, Dongle d, Dongle alt) 
  throws ConnectionException
{
  log.debug("Attempting connection of dongle " + d + " to widget " + w);
  Connection c;
  try {
    c = w.connect(d);
  } catch(ConnectionException ex) {
    log.warn("Connection failed; attempting alternate dongle " + d, ex);
    c = w.connect(alt);
  }
  log.debug("Connection succeeded: " + c);
  return c;
}

In Java, each of the log statements creates a new StringBuilder, and invokes the toString() method on each object concatenated to the string. These toString() methods, in turn, are likely to create StringBuilder instances of their own, and invoke the toString() methods of their members, and so on, across a potentially large object graph. (Before Java 5, it was even more expensive, since StringBuffer was used, and all of its operations are synchronized.)

This can be relatively costly, especially if the log statement is in some heavily-executed code path. And, written as above, that expensive message formatting occurs even if the logger is bound to discard the result because the log level is too high.

This leads to the introduction of guard statements of the form:

  if (log.isDebugEnabled())
    log.debug("Attempting connection of dongle " + d + " to widget " + w);

With this guard, the evaluation of arguments d and w and the string concatenation is performed only when necessary.

A solution for simple, efficient logging

However, if the logger (or a wrapper that you write around your chosen logging package) takes a formatter and arguments for the formatter, the message construction can be delayed until it is certain that it will be used, while eliminating the guard statements and their cyclomatic complexity.

public final class FormatLogger
{

  private final Logger log;

  public FormatLogger(Logger log)
  {
    this.log = log;
  }

  public void debug(String formatter, Object... args)
  {
    log(Level.DEBUG, formatter, args);
  }

  … &c. for info, warn; also add overloads to log an exception …

  public void log(Level level, String formatter, Object... args)
  {
    if (log.isEnabled(level)) {
      /* 
       * Only now is the message constructed, and each "arg"
       * evaluated by having its toString() method invoked.
       */
      log.log(level, String.format(formatter, args));
    }
  }

}

class MyClass 
{

  private static final FormatLogger log = 
     new FormatLogger(Logger.getLogger(MyClass.class));

  Connection connect(Widget w, Dongle d, Dongle alt) 
    throws ConnectionException
  {
    log.debug("Attempting connection of dongle %s to widget %s.", d, w);
    Connection c;
    try {
      c = w.connect(d);
    } catch(ConnectionException ex) {
      log.warn("Connection failed; attempting alternate dongle %s.", d);
      c = w.connect(alt);
    }
    log.debug("Connection succeeded: %s", c);
    return c;
  }

}

Now, none of the cascading toString() calls with their buffer allocations will occur unless they are necessary! This effectively eliminates the performance hit that led to the guard statements. One small penalty, in Java, would be auto-boxing of any primitive type arguments you pass to the logger.

The code doing the logging is arguably even cleaner than ever, since untidy string concatenation is gone. It can be even cleaner if the format strings are externalized (using a ResourceBundle), which could also assist in maintenance or localization of the software.

Further enhancements

Also note that, in Java, a MessageFormat object could be used in place of a "format" String, which gives you additional capabilities such as a choice format to handle cardinal numbers more neatly. Another alternative would be to implement your own formatting capability that invokes some interface that you define for "evaluation", rather than the basic toString() method.

Do you find cyclomatic complexity a useful measure

We refactor mercilessly, and use Cyclomatic complexity as one of the metrics that gets code on our 'hit list'. 1-6 we don't flag for complexity (although it could get questioned for other reasons), 7-9 is questionable, and any method over 10 is assumed to be bad unless proven otherwise.

The worst we've seen was 87 from a monstrous if-else-if chain in some legacy code we had to take over.