How can I reduce the Cyclomatic Complexity of this?

Question

I have a method that receives an Object and does something based on what type of object it detects:

void receive(Object object) {
    if (object instanceof ObjectTypeA) {
        doSomethingA();
    }
    else {
        if (object instanceof ObjectTypeB) {
            doSomethingB();
        }
        else {
            if (object instanceof ObjectTypeC) {
                doSomethingC();
            }
            else {
                if (object instanceof ObjectTypeD) {
                    doSomethingD();
                }
                else {
                    // etc...
                }
            }
        }
    }
}

How can I reduce the Cyclomatic Complexity? I searched around but couldn't find anything too useful.

Solution 1

Can't you leverage an object-oriented approach for this? Create an interface that has the doSomething() method then create subclasses that implement the desired behavior? Then calling object.doSomething() would execute the appropriate behavior?

Solution 2

Cyclomatic complexity is a measure based on graph structure of the code. Specifically, it is based on the number of possible paths through the code; see here for more details. While there is a correlation between CC and what a typical programmer would view as code complexity, they are not the same thing. For instance:

• CC takes no account of the semantics of the code; e.g. what such-and-such method that is being called does, or the mathematical properties of an algorithm.

• CC takes no account of design and coding patterns. So, something that CC says is complex may well be simple to someone who understands the pattern being used.

You could say that the relationship between CC and real code complexity is like the relationship between IQ and real intelligence.

Thus, Cyclomatic Complexity should be treated as an indicator of where the complex parts of your code is ... not as a true measure of complexity, or of code quality. Indeed, highly complex code is not necessarily poor quality. Often, the complexity is inherent, and trying to get rid of it only makes matters worse.

---

In this particular example, the high CC measure does not correspond to something that would cause a typical programmer any difficulties. The best answer (IMO) is to leave the method alone. Chalk it up as a false positive.

Solution 3

void receive(ObjectTypeA object) {
        doSomethingA();
}

void receive(ObjectTypeB object) {
        doSomethingB();
}

void receive(ObjectTypeC object) {
        doSomethingC();
}

...

// Your final 'else' method
void receive(Object object) {
        doSomethingZ();
}

Comments

[Vinod R]

It will be difficult to write the unit test cases if the code is written this way. One more data type you add will result in one more step in the ladder to be added and one more test. OO allows abstraction to take care of this, if your approach is to be taken it is better we write in C instead of Java.

[Nicolas Bousquet]

Vinod R, OOP added a new choice, using polymorphism. This is not to say this is always the best solution. When the possible methods are fixed (and small), and the subtypes can vary a lot, using OOP and polymorphism is a good solution. But if on the contrary, your set of subtypes is limited but the possible thing you want to do with them is unbounded, a "switch case" approach is better. In this language with pattern matching are the best, not specially OOP ones. We could benefit from an enum through. As of unit tests, whatever the solution the number of case to test remain the same.

[Nicolas Bousquet]

This would work, but if this only to reduce cyclomatic complexity this is bad. From the exemple it seem that this is the callback of a listener. Typically and object doesn't contains one method as per possible listener specific usage.

[laz]

I wasn't approaching it from a cyclomatic complexity perspective, so in that sense I didn't answer the question. However when I see many conditionals all using instanceof I think polymorphism should seriously be considered, if for nothing else then to create more maintainable code. That's what cyclomatic complexity measurements are meant to assist with anyway.

[Nicolas Bousquet]

Well think about any external library for exemple. You have a case depending of the type of whatever class hierarchy you use (like widget type in SWING). This would be very bad pratice to modify the external library for this. And the external library can't know in advence all the things anybody will ever do with it. So it define generic listener and you can check event or originator type. Even if this your own code, seperation of concern principe will make you avoid adding too much unrelated methods on objects just for the matter of removing a harmless switch statement.

[mattnz]

Vinof R - Agreed, In some cases the pure OO solution, while preferable, is not possible. The question did not contain enough information to indicate a pure OO solution would be useable. Closing the switch/case with an else/default, allows the detection of unimplemented cases.

[davidjnelson]

this might be acceptable in a prototype where no one else never had to read the code, and bugs were acceptable. otherwise, use polymorphism.

[davidjnelson]

reducing cyclomatic complexity reduces bugs. scientific fact: en.wikipedia.org/wiki/…

[davidjnelson]

reducing cyclomatic complexity reduces bugs. scientific fact: en.wikipedia.org/wiki/… Also, if you want to see how code quality affects the bottom line of a business, check this out: ibm.com/developerworks/rational/library/4995.html

[Vladimir Dyuzhev]

Science by your link is, as they themselves say, "inconclusive".

[mattnz]

@devadvocate : Is that so - Read the second paragraph : "However, studies that control for program size (...) are generally less conclusive, with many finding no significant correlation....."

[Torsten]

I agree with your general definition for CC. However, I partially disagree with your specific answer for this question. The code is indeed easy to read for an average programmer. However, using an interface as stated by laz, would clean up the code. You wouldn't be able to see what's happening with one look. But that's typical for refactoring large methods into smaller ones.

[Seph]

@davidjnelson from your link "The essence of this observation is that larger programs tend to have more defects.". This means that Cyclomatic Complexity may very well have only the same predictive ability as lines of code.

[Stephen C]

@Torsten - You misunderstand what I'm saying. I'm not saying that you can't reduce the CC here. This example should not cause a programmer of average ability any problems, no matter what the CC is saying. And since the real goal is to make the code readable, no change is warranted. "Cleaning up the code" is only worth doing if there are readability issues.

[Stephen C]

@davidjnelson - Here's what the authority you cited (Wikipedia) says now: "Thus, reducing the cyclomatic complexity of code is NOT proven to reduce the number of errors or bugs in that code."