2
\$\begingroup\$

I am creating an application that deals with cars and orders.

A order contains a car and other information (BUYER, OWNER, ...), which constructs an orderObject.

A car has around 80 different attributes (TYP_WHEEL, TYP_DOOR, QUANTITY_WHEELS, ...) for the different car-parts.

I get the car-attributes and their values from a JSON string and put them into a map named allCarAttributes.

The JSON has this structure:

result: 
    data[]:
        characteristics_0[]: …
            …
        characteristics_1[]: …
            …
            …
            …
        characteristics_N[]: …

Each characteristics-array contains some attributes:

"result": {
    "data": [
        {
            "characteristics": [
                {
                    "description": "Car ceiling color",
                    "id": "COL_CAR_CEILING",
                    "value": "F8",
                    "valueDescription": "F8 - White"
                },
                {
                    "description": "Color Car Finishes",
                    "id": "COL_CAR_ACCESSORIES",
                    "value": "T",
                    "valueDescription": "ST- Stainless Steel"
                },
                {
                    "description": "Car flooring color",
                    "id": "COL_CAR_FLOORING",
                    "value": "RD10",
                    "valueDescription": "RD10, grey"
                },
                (...)

Then I go through this map and based on the attribute name I set the respective value in the orderObject.

My problem: Since the methods to set the values in the orderObject each have different names, I am not able to find a more efficient way than using a switch-case with 80 cases.

for (Map.Entry<String, String> attribute : allCarAttributes.entrySet()) {

        switch (attribute.getKey()) {
        case "TYP_DOOR":
            orderObject.setDoor(attribute.getValue());
            break;
        case "TYP_WHEEL":
            orderObject.setWheelType(attribute.getValue());
            break;
        case "QANTITY_WHEELS":
            orderObject.setWheelNumber(attribute.getValue());
            break;

        (...)

        default:
            break;
        }
    }

The orderObject-class simply has several fields and setters/getters for them:

private String car_width;
private String car_depth;
private String car_height;
private String car_weight;
(...)

public String getCar_width() {
    return car_width;
}

public void setCar_width(String car_width) {
    this.car_width = car_width;
}

How can I get rid of this huge switch-case?

Should I get rid of it at all? The switch-case works fine as it is, it is just very long and ugly.

\$\endgroup\$
  • \$\begingroup\$ I get the car-attributes and their values from outside Could you clarify how you're getting this data? Why are you putting them into a map? I am sure there are better ways than putting them into a map, only to read its content back in a non-type safe way. \$\endgroup\$ – Tunaki Oct 5 '16 at 8:06
  • \$\begingroup\$ @Tunaki: I get the data from a JSON, which contains the car-parts as attributes, but no orders. From the JSON I put the attributeName as the key and the attributeValue as the value for the map. Could you explain why this is non-type safe? ... I've also added the information about the JSON back into my question. \$\endgroup\$ – hamena314 Oct 5 '16 at 8:23
  • \$\begingroup\$ Non-type safe, because you're now only dealing with String values. car_width for example is not a String but a number (maybe int), safe for quantities. Can you post what your JSON looks like? Parsing it into an Object up-front using Gson or other JSON libraries will be a lot simpler. \$\endgroup\$ – Tunaki Oct 5 '16 at 8:39
  • 1
    \$\begingroup\$ I edited the title in Rev 2 for several reasons. 1) It's the site standard to state the task being performed — see How to Ask and the rationale. 2) Titles like "refactor long switch statement" are common requests that would fail to distinguish your question if everyone used the same kind of title. 3) On this site, we review code that performs a specific task; we don't dispense generic advice for hypothetical situations. A guide to CR for Stack Overflow users may clear up some misconceptions. \$\endgroup\$ – 200_success Oct 5 '16 at 8:42
  • \$\begingroup\$ @Tunaki: I've added the JSON. Since the orderObject does not only contain information about the car-parts but several other outside I did not see how I could directly create an orderObject via Gson. Therefore I create it "step-by-step" from the JSON + the other information. \$\endgroup\$ – hamena314 Oct 5 '16 at 8:48
1
\$\begingroup\$

This is the typical problem the relational community (relational databases) answers with OR-Mappings. As we do not have a relational datastructure more an object structure the mapping impedance is less but it remains.

But as we have to do a mapping it is not the question IF but the question WHERE the mapping is neccessary. Finally you have ALWAYS a big switch-case-statement. It may not be obvious and maybe it is not done all by yourself but there is no way around. You cannot escape the mapping but you can get help with it.

A pragmatic way would be to use the "org.json" API, Here is the Maven dependency:

<dependency>
    <groupId>org.json</groupId>
    <artifactId>json</artifactId>
    <version>1.5-20090211</version>
</dependency>

With this library your mapping will be a little be easier. You can parse the json string:

JSONObject carInfoAsJsonObject = new JSONObject(jsonString);

Then you can wrapp this JSONObject into your own object and have the getters returning their values directly from the JSONObject:

class OrderObject {
    public String getCar_width() {
        return carInfoAsJsonObject.getString("CAR_WIDTH");
    }
}

Here you do not have a switch-case have you? Of course you have. But it is hidden in the structure we chose and not formulated with the JAVA language element "switch-case".

Another way is to let a library make the mapping decision. A good library is "Jackson". Here is the Maven dependency:

<dependency>
    <groupId>org.codehaus.jackson</groupId>
    <artifactId>jackson-mapper-asl</artifactId>
    <version>1.9.13</version>
</dependency>

The Jackson way is "mapping by name"-convention. Your keys must be named like the attributes of your JAVA Object that should follow the bean convention.

The central clas is the ObjectMapper you have to instantiate:

ObjectMapper mapper = new ObjectMapper();

Then you may have the OrderObject attribute names synchronized:

class OrderObject {
    private String CAR_WIDTH;
    // getter and setter
}

After that you can instantiate an object of the type OrderObject:

OrderObject orderObject = objectMapper.readValue(carInfoAsJsonString, OrderObject.class);
\$\endgroup\$
  • \$\begingroup\$ Altough I am done with this project, the Jackson-Mapper sounds like the solution with the least needed manual input. Since some parts later might change, this would require only a few changes in the OrderObject without touching the mapping -> accepted. \$\endgroup\$ – hamena314 Feb 3 '17 at 12:38
0
\$\begingroup\$

You can put you String to Function mapping into HashTable and you can just create a method that assigns the value. Here is the String to Function Mapping Example from stackoverflow:

interface ProcessingMethod {
   String method();
}

Map<String, ProcessingMethod> methodMap = new HashMap<String, ProcessingMethod>();
methodMap.put("abc", new ProcessingMethod() {
   String method() { return "xyz" }
});
methodMap.put("def", new ProcessingMethod() {
   String method() { return "uvw" }
});

methodMap.get("abc").method();
\$\endgroup\$
  • \$\begingroup\$ Thanks for your answer, but I am not sure I understand what's happening here. Could you explain this a bit more and maybe fill in "TYP_DOOR" and .setDoor()? Because I cant follow why you return a String in the method. I also dont get why you use an Interface here? Also wouldn't I have to use 80 times methodMap.get("TYP_DOOR").method() ... methodMap.get("QUANTITY_WHEELS").method() ... ? \$\endgroup\$ – hamena314 Oct 4 '16 at 16:47
  • \$\begingroup\$ In this case, your methodMap entry might be methodMap.put("TYP_DOOR", () -> orderObject.setDoor(attribute.getValue()));. Your method would then get the function from the map based on the key, then call it. \$\endgroup\$ – Joe C Oct 4 '16 at 21:43
  • \$\begingroup\$ @JoeC: Thanks, it's a bit clearer now, but I dont understand everything: If I need to write 80 calls with methodMap.put("ATTRIBUTENAME", () -> orderObject.setATTRIBUTE(attribute.getValue()))); what is the advantage over the switch-casesince this seems even longer and more complex? Also why did you use a lambda-expression, couldnt I write methodMap.put("attributeName", orderObject.setATTRIBUTE(attribute.getValue())));? \$\endgroup\$ – hamena314 Oct 5 '16 at 8:40
  • \$\begingroup\$ I've found the answer from StackOverflow that Ilgorbek Kuchkarov mentioned: stackoverflow.com/a/2745851/1368690 ... sadly, it has no further explanation as to what the advantage is over using a switch-case. \$\endgroup\$ – hamena314 Oct 5 '16 at 12:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.