Best way to avoid multiple parallel if else loop in java 8

Question

What is the best way to avoid multiple parallel if-else loop. I tried with switch statement as well, but again that doesn't look readable. I have hundreds of such statements:

public static Map getKqvSecureNodeResponse(Sample secureNodeData, Map<String, Object> map) {
    if(map.containsKey(Constants.NAME_KQV)) {
        map.put(Constants.NAME_KQV, secureNodeData.getNodename());
    }
    if(map.containsKey(Constants.SPOV)) {
        map.put(Constants.SPOV, secureNodeData.getOverride());
    }
    if(map.containsKey(Constants.SPEP)) {
        map.put(Constants.SPEP, secureNodeData.getEnabledProtocol());
    }
    if(map.containsKey(Constants.SPTO)) {
        map.put(Constants.SPTO, secureNodeData.getAuthTimeout());
    }
    if(map.containsKey(Constants.TLCN)) {
        map.put(Constants.TLCN, secureNodeData.getCommonName());
    }
    if(map.containsKey(Constants.SEDT)) {
        map.put(Constants.SEDT, secureNodeData.getEncryptData());
    }
    if(map.containsKey(Constants.TLCF)) {
        map.put(Constants.TLCF, secureNodeData.getKeyCertLabel());
    }
    if(map.containsKey(Constants.TLCL)) {
        map.put(Constants.TLCL, secureNodeData.getCipherSuites());
    }
    return map;
}

Please note that I have to invoke different getter of secureNodeData for every check.

But I have to invoke diffrent getter depending on the if clause. `secureNodeData.getNodename()` would be called for `map.containsKey(Constants.NAME_KQV)` only. — Abhishek, Sep 18 '18 at 09:43
`switch` might help the branches look cleaner, yet must revisit your design probably as pointed by @Ravi. Also, the part `Map` is way to generic IMO. — Naman, Sep 18 '18 at 09:45
Could you provide the source code of the `Sample` class? The solution might be there if revisited a little. — Robert Kock, Sep 18 '18 at 09:50
@nullpointer I vote to reopen this question because the specific code here can be rewritten using method references (Java 8), unlike the other questions that are too general for such answer. — Pavel Horal, Sep 18 '18 at 09:55
@Antoniossss OP validates the condition `map.containsKey`, so it should be `computeIfPresent` (they'd rather override the value) — Andrew Tobilko, Sep 18 '18 at 10:16

Andrew Tobilko · Answer 1 · 2018-09-18T12:35:54.563

For each Constants value (e.g. Constants.NAME_KQV), you can provide a Function<Sample, Object> (e.g. sample -> sample.getNodename()).

If you organised it in a structure like Map or enum (here, I used a enum), you could end up with a simple loop:

public static Map<String, Object> getKqvSecureNodeResponse(Sample secureNodeData, Map<String, Object> map) {
    for (Constant constant : Constant.values()) {
        final String name = constant.getName();
        if (map.containsKey(name)) {
            map.put(name, constant.getFunction().apply(secureNodeData));
        }
    }
    return map;
}

The enum was defined as:

enum Constant {
    NAME_KQV(Constants.NAME_KQV, Sample::getNodename);
    // other definitions

    final String name;
    final Function<Sample, Object> function;

    Constant(String name, Function<Sample, Object> function) {
        this.name = name;
        this.function = function;
    }

    public String getName() {
        return name;
    }

    public Function<Sample, Object> getFunction() {
        return function;
    }
}

It seems like this method does a lot. (1) It's unclear why it overrides existing values. (2) The method name is obscure. (3) You are using a raw Map, replace it with Map<String, Object> at least, and figure out how to substitute the Object part. (4)

I feel rethinking the design would help much more than the above approach and these small corrections.

`null` values will be filtered out by `computeIfPresent`, won't they? — Andrew Tobilko, Sep 18 '18 at 10:55
@AndrewTobilko I will try this code and accept it as answer soon. Thanks much for this answer. I can't change the code of Sample class as we're using some dependent jar and I can't modify it. — Abhishek, Sep 18 '18 at 11:21
@AndrewTobilko if there are keys mapping to `null`, `computeIfPresent` will treat them as absent, whereas `containsKey` will report the keys as present, so this solution will have a different behavior than the original code, if the incoming map has `null` values. And since this method is supposed to replace the values, this might not be too far-fetched. — Holger, Sep 18 '18 at 11:34

Pavel Horal · Answer 2 · 2018-09-18T14:25:20.443

You can try to take advantage of method references:

public static Map getKqvSecureNodeResponse(Sample node, Map<String, Object> map) {
    applyParam(Constants.NAME_KQV, map, node::getNodename);
    applyParam(Constants.SPOV, map, node::getOverride);
    // ...
}

public static void applyParam(String key, Map<String, Object> data, Supplier<Object> getter) {
    if (data.containsKey(key)) {
        data.put(key, getter.get());
    }
}

Alternatively you can use Function references that are instance independent:

private static final Map<String, Function<Sample, Object>> MAPPING;
static {
    MAPPING = new LinkedHashMap<>();
    MAPPING.put(Constants.NAME_KQV, Sample::getNodename);
    MAPPING.put(Constants.SPOV, Sample::getOverride);
}

public static Map getKqvSecureNodeResponse(Sample node, Map<String, Object> map) {
    for (String key : MAPPING.keySet()) {
        if (map.containsKey(key)) {
            map.put(key, MAPPING.get(key).apply(node));
        }
    }
}

There are many ways how you can approach your specific use case, but method references in general makes developer's life much much easier.

Best way to avoid multiple parallel if else loop in java 8

2 Answers2