Apache POI and cyclomatic complexity of code

Question

I have an Excel file, which I am reading using Apache POI APIs; which gives me control over cells/columns in my Excel.

I have around 17 columns in my Excel and these are with different datatypes. I want to populate a model object from these column values. So, I came up with something like below to populate my model object:

private WHTApps populateModel(WHTApps whtApp, int columnIndex, Object cellValue) {
 switch (columnIndex) {
 case 0:
 whtApp.setVendorCode((int) cellValue);
 break;
 case 1:
 whtApp.setVendorName((String) cellValue);
 break;
 case 2:
 whtApp.setCountry((String) cellValue);
 break;
 case 3:
 whtApp.setApplicationReceivedFromUSDate((Date) cellValue);
 break;
 case 4:
 whtApp.setContractNumber((String) cellValue);
 break;
 case 5:
 whtApp.setContractEffectiveDate((Date) cellValue);
 break;
 case 6:
 whtApp.setContractExpirationDate((Date) cellValue);
 break;
 case 7:
 whtApp.setAutomaticRenewal((String) cellValue);
 break;
 case 8:
 whtApp.setReducedRate((int) cellValue);
 break;
 case 9:
 whtApp.setForm17((String) cellValue);
 break;
 case 10:
 whtApp.setResidencyCertIssueDate((Date) cellValue);
 break;
 case 11:
 whtApp.setTaxAuthoritySubmissionDate((Date) cellValue);
 break;
 case 12:
 whtApp.setTaxAuthorityAcceptanceDate((Date) cellValue);
 break;
 case 13:
 whtApp.setStatus((String) cellValue);
 break;
 case 14:
 whtApp.setForm17Expiration((Date) cellValue);
 break;
 case 15:
 whtApp.setComments((String) cellValue);
 break;
 case 16:
 whtApp.setRemarks((String) cellValue);
 break;
 case 17:
 whtApp.setCategory((String) cellValue);
 break;
 default:
 }
 return whtApp;
 }

But, this gives rise to a really high cyclomatic complexity value of 19.

Now how do I reduce the cyclomatic complexity as I have 17 columns which I need to populate in my model object?

• default locale: I don't really understand how this method is supposed to be used. The strange thing is that it returns a new object for each column. For example, subsequent calls: WHTApps app1 = populateModel(0, "code"); and ` WHTApps app2 = populateModel(1, "name");` will return different objects. Is that correct?
• Akshay Lokur: You are correct. In fact I had fixed this just some time back, now the method does not return new object for each call. However, any thoughts on cyclomatic complexity reduction though? Thanks

Answer 1

If you want to create an object from Excel row you can change the method's signature to accept an array of values:

private WHTApps createModelFromValues(Object[] values) {
 WHTApps whtApp = new WHTApps();
 whtApp.setVendorCode((int) values[0]);
 whtApp.setVendorName((String) values[1]);
 ....
 whtApp.setCategory((String) values[17]);
 return whtApp;
}

This method is not as complex as the first one and is easier to test. Client code will have to populate an array of values to pass.

You should consider adding some kind of error checking: validate values array before processing it. Check for length and data type errors. You can create a custom exception class that will clearly show where the problem is.

Probably, you can make this code more flexible by adding a separate mapper class that will set properties using reflection. I'm not sure if it's worth the effort in this particular case, though.

Answer 2

If (and only if) you call this method from a single location, and that calling method uses a for loop over all the columns, you can replace both the for loop and theswitch with a simple sequence of statements.

In all other cases, don't worry about the reported complexity. The human complexity is much less than 17 since it is a switch statement where all the branches look essentially the same, so it's very simple to understand.

Just ignore the warning and continue with other, more important things.

Answer 3

What you use there is a jumptable. Since Java 8 you can go so far as to move these jumptables into runtime, which reduces performance, but increases understandability.

Since you use contiguous indices, you might as well make that jumptable an array instead of a Map<Integer, ...>. The result looks like this:

private static final BiConsumer<WHTApps, Object>[] columnFunctions = {
 (whtApp, cellValue) -> whtApp.setVendorCode((int) cellValue),
 (whtApp, cellValue) -> whtApp.setVendorName((String) cellValue),
 (whtApp, cellValue) -> whtApp.setCountry((String) cellValue),
 // ...
 (whtApp, cellValue) -> whtApp.setCategory((String) cellValue)
};
private WHTApps populateModel(WHTApps whtApp, int columnIndex, Object cellValue) {
 columnFunctions[columnIndex].accept(whtApp, cellValue);
 return whtApp;
}

Do note that default locale's answer addresses the actual underlying problem. Instead of piece-by-piece updating a whtApp you should be constructing it from a full row of information. That should reduce complexity quite a bit