Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Refactor multiple cases from switch

Tags:

java

TestDTO testDTO = new TestDTO();
    
for (Object attribute : row.getAttributes()) {
    switch (attribute) {
    case "CATEGORY":
        testDTO.setCategory((String) attribute);
        break;
    case "DESCRIPTION":
        testDTO.setDescription((String) attribute);
        break;
    case "NOTE":
        testDTO.setNote((String) attribute);
        break;
    case "FEATURES":
        testDTO.setFeatures((String) attribute);
        break;
    case "INDICATOR":
        testDTO.setIndicator((String) attribute);
        break;
    case "LABEL":
        testDTO.setLabel((String) attribute);
        break;
    case "TYPE":
        testDTO.setType((String) attribute);
        break;
    default:

    }
}

As you can see in above code, we are using multiple case for setting data. Code is working fine.

Is there any way for reducing multiple cases for setting those data.

In the above code, the problem is maintainability. Because suppose if we have 30 fields, then we need to put 30 cases for that.

Is there any other way to achieve the same?

like image 800
Shiladittya Chakraborty Avatar asked Jul 31 '20 12:07

Shiladittya Chakraborty


2 Answers

Without refactoring you cannot do anything really helping the situation. Also you will need to add specific code for every field anyway - this is obvious

In abstract situation what you could do would be to implement factory or strategy pattern and e.g. register proper handlers for every type of attribute - something like

Map<Object, BiConsumer<TestoDTO, Object>> handlers; // then you can add for example handlers.put("TYPE", (d, a) -> d.setType(a))

And just iterate over attributes

row.getAttributes().forEach(a -> handlers.get(attribute).accept(dto, a)); // ofc you need to handle all situation like NPE, no key etc

In scope of mapping objects you could use some existing tool like ObjectMapper or ModelMapper because it's quite possible that these tools will resolve your issue out of the box

Last and least (:)) solution is to use some reflection, map attribute to field name, extract setter... Don't do this :) it's filthy, insecure, hard to write and understand - will cause many issues you will regret but because it's an option I'm mentioning this

like image 127
m.antkowicz Avatar answered Oct 31 '22 07:10

m.antkowicz


For a robust solution you can also build your association using enumerated types and method references, and conveniently encapsulate the map into a single type. Plus, it's pretty obvious how to add new fields:

enum DTOMap
{
    CATEGORY(TestDTO::setCategory),
    DESCRIPTION(TestDTO::setDescription); 
    
    private final BiConsumer<TestDTO, String> attributeConsumer;
    
    private DTOMap(BiConsumer<TestDTO, String> attributeConsumer) {
        this.attributeConsumer = attributeConsumer;
    }
    
    public static void execute(TestDTO testDTO, Object attribute) {
        String attributeAsString = (String) attribute;
        DTOMap.valueOf(attributeAsString.toUpperCase()).attributeConsumer.accept(testDTO, attributeAsString);
    }
}

With this your switch statement can be reduced to a single line:

for (Object attribute : row.getAttributes()) {
    DTOMap.execute(testDTO, attribute);
}
like image 24
SDJ Avatar answered Oct 31 '22 08:10

SDJ