Skip to content
Tags

Java: Reducing Cyclomatic Complexity with Builder Pattern

by on October 16, 2012

If you get used to Guava Libraries as I did you will soon like to adopt some commonly used patterns. One pattern you find throughout Guava is the Builder Pattern: The builder collects required information to eventually create an object based on all these information.

What you might not know: This is a great way to reduce cyclomatic complexity reported by some static code analysis tools. Just as rough “definition”: A high cyclomatic complexity is a hint that it is hard to understand the code.

Here is a rough sketch of code which had a bad cyclomatic complexity:

Descriptor parse(String descriptor) {
  checkNotNull(descriptor, "Descriptor must not be null.");
  final String[] elements = checkDescriptor(descriptor);
  String key = null;
  String alias = "";
  Pattern somePattern = DEFAULT_VERSION_PATTERN;
  String replacement = "$0";
  for (int i = 0; i < elements.length; i++) {
    String el = elements[i];
    switch (i) {
      case KEY_POS:
        key = checkKey(el, descriptor);
        break;
      case ALIAS_POS:
        if (!el.isEmpty()) {
          alias = el;
        }
        break;
      case PATTERN_POS:
        if (!el.isEmpty()) {
          somePattern = checkPattern(el, descriptor);
        }
        break;
      case REPLACEMENT_POS:
        if (!el.isEmpty()) {
          replacement = el;
        }
        break;
      default:
        throw IllegalStateException(...);
    }
  }
  if (key == null) {
    throw IllegalStateException(...);
  }
  return new Descriptor(key, alias, somePattern, replacement);
}

As you see there is a switch statement and some if-statements… and even some hidden in the different check-methods. Although it’s only 37 lines of code it’s already hard to follow the control flow.

Now how to get rid of this? One approach is using the builder-pattern: Let a builder collect all information and eventually create the Descriptor object. Here is the refactored method; I think you can guess the implementation of the builder:

Descriptor parse(String descriptor) {
  checkNotNull(descriptor, "Descriptor must not be null.");
  final String[] elements = checkDescriptor(descriptor);
  builder = new DescriptorBuilder();
  for (int i = 0; i < elements.length; i++) {
    String el = elements[i];
    switch (i) {
      case KEY_POS:
	    builder.setKey(el);
        break;
      case ALIAS_POS:
	    builder.setAlias(el);
        break;
      case PATTERN_POS:
	    builder.setPattern(el);
        break;
      case REPLACEMENT_POS:
	    builder.setReplacement(el);
        break;
      default:
        throw IllegalStateException(...);
    }
  }
  return builder.build();
}

Now you might stumble across the same effect I often experience: Isn’t there even more place for improvement? Now that it is easier to read the code, it is easier to understand the code – and to detect patterns.

Common builder classes always return self-references for their setters. This allows to use chained calls. And knowing that checkDescriptor() also checks that the number of elements is exactly 4 you can now just type:

Descriptor parse(String descriptor) {
  checkNotNull(descriptor, "Descriptor must not be null.");
  final String[] elements = checkDescriptor(descriptor);
  return new DescriptorBuilder()
    .setKey(elements[KEY_POS])
	.setAlias(elements[ALIAS_POS])
	.setPattern(elements[PATTERN_POS])
	.setReplacement(elements[REPLACEMENT_POS])
	.build();
}

Guess what? From 37 lines of code we are down to 10. 10 lines of code you need to read to understand what the parse() does. Of course the complexity is now hidden in the builder but at first glance someone needs only to understand 10 lines of code to get some kind of summary.

And even more: You also increased the testabiliy of the code. You can now write extra tests for the builder and only have to do a minimum effort to also test the parse-method, perhaps even mocking the builder inside.

Do you have other patterns to fight the cyclomatic complexity?

One Comment
  1. rolfwatermann permalink

    I think you mix up some independent issues.

    Your initial goal is reduction of cyclomatic complexity. You achieve it basically by delegating the argument checking to an other object. This is a good approach, but it is independent of the builder pattern. You can also do it with the “afterPropertiesSet” pattern and instantiate the target object directly. The advantages of the builder (aka factory) pattern are of course undisputed, but are not related to cyclomatic complexity.

    In the second half you combine elimination of the switch statement with chained calls, which is also independent of each other. While I like the simplification of the switch statement, I will start a discussion about chained expressions in a new thread…

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s