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.

Was it helpful?

Solution

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?

OTHER TIPS

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.

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

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

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

...

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

Why the need to reduce the complexity? It is a simple enough pattern that any competent developer would see it as a trivial function.

I would probably write it this way

    if (object instanceof ObjectTypeA) 
    {
        doSomethingA();
    }
    else if (object instanceof ObjectTypeB) 
    {
        doSomethingB();
    }
    else if (object instanceof ObjectTypeC) 
    {
        doSomethingC();
    }

If it is to to meet some esoteric need to "CC must be less than x", then the overarching rule that the standards are there to ensure maintainable code would mean this is acceptable no matter how high the CC gets.

Never had a goal to "reduce the cyclomatic complexity", though there were times when I was paid by LOC.

You code is "good enough". My eyes stumble on brackets, so I'd sacrifice a bit of performance and did the following (providing types A, B and so on are not in the same hierarchy):

receive(Object object) {
    if (object intanceof ObjectTypeA) doSomethingA();
    if (object instanceof ObjectTypeB) doSomethingB();
    ...

or (if they are in the same hierarchy):

receive(Object object) {
    if (object intanceof ObjectTypeA) { doSomethingA(); return; }
    if (object instanceof ObjectTypeB) { doSomethingB(); return; }
    ...

Don't know if it would reduce the cyclomatic thingy, and couldn't care less.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top