Franz Kafka
Franz Kafka

Reputation: 10841

McCabe Cyclomatic Complexity for switch in Java

I am using a switch statement with 13 cases, each case only has an one line return value.

McCabe paints this in red. Is there an easier way to write a big switch statement? It doesn't seem complex to read, but I don't like the default setting turning red. If other people use the same tool on my code and see red stuff they might think I'm stupid :-)

Edit: I'm mapping different SQL-Types to my own more abstract types, therefore reducing the total amount of types.

case Types.TIME:
    return AbstractDataType.TIME;
case Types.TIMESTAMP:
    return AbstractDataType.TIME;
case Types.DATE:
    return AbstractDataType.TIME;
case Types.BIGINT:
    return AbstractDataType.NUMERIC;
case Types.DECIMAL:
    return AbstractDataType.NUMERIC;

and so on...

Upvotes: 9

Views: 2873

Answers (3)

Michael Wiles
Michael Wiles

Reputation: 21184

+1 for the Map idea...

Something like this:

Initialize the map

Map<Types, AbstractDataType> map = new HashMap<Types, AbstractDataType>();
map.put(Types.TIME, AbstractDataTypes.TIME);
// and so on

Then in your code simple do

return map.get(sqlTimeType);

An even better solution however would be to include this mapping in the enum itself so you would be able to do assuming you don't have control over the Sql enum types...

AbstractDataTypes.fromSqlType(timeType);

and if you do:

sqlTimeType.getAbstractType();

Encapsulated and Re-usable :-)

Upvotes: 3

Patrick from NDepend team
Patrick from NDepend team

Reputation: 13842

You are using code to express what really is data. Just use an enum map or define once for all a constant Dictionary. This way, you are just parametrizing a simple and generic correspondence algorithm, instead of writing a long switch case.

Upvotes: 7

arcy
arcy

Reputation: 13133

I don't know that much about McCabe tools. One of the things Cyclomatic complexity considers is multiple exit points.

I like the EnumMap idea.

If a switch is going to be used, you could have a result variable and do away with all the return statements. You can also collapse all the source values that have the same result type:

result = null;

case Types.TIME:
case Types.DATE:
case Types.TIMESTAMP: result = AbstractDataType.TIME

// etc.

return result;

I think this reduces the cyclomatic complexity, regardless of what anyone thinks about it as style. And it is a different way to write the statement, though whether it is easier you should judge.

Upvotes: 6

Related Questions