Refactor multiple cases from switch

user_4969916 0
Viewed 293
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?

java
3 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

user_41534260

You can use reflection to refactor it like below:

TestDTO testDTO = new TestDTO();

for (Object attribute : row.getAttributes()) {

    Method method = testDTO.getClass().getMethod("set" + capitalize((String) attribute), String.class);
    method.invoke(testDTO, (String) attribute);
}

The capitalize func:

public String capitalize(String string) {

    return string.substring(0, 1).toUpperCase() + string.substring(1).toLowerCase();
}
user_111566660

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);
}
user_115957280
Related