2

I have a code block that redirects a Cassandra query to different Cassandra tables based on the available parameters such that I check multiple logical conditions inside multiple if conditions. I'm trying my hand at java 8 and looking to reduce these conditions to lambda expressions. Here's how the code currently looks,

String processTable(String cid, String postcode, String[] ratingvalue, String ratingType) {

    String table = "";
    if (postcode != null && ratingvalue == null) {
        table = cassconf.getTable1();
    }
    if (postcode != null && ratingvalue != null) {
        table = cassconf.getTable2();
    }
    if (cid != null && ratingvalue == null) {
        table = cassconf.getTable3();
    }
    if (cid != null && ratingvalue != null) {
        table = cassconf.getTable4();
    }
    if (cid != null && postcode != null && ratingvalue == null) {
        table = cassconf.getTable5();
    }
    if (cid != null && postcode != null & ratingvalue != null) {
        table = cassconf.getTable6();
    }
    return table;

}

My problem is even if I store the arguments in a map and filter the unavailable values from the stream, I don't know how to return the final value of the table based on these 6 different conditions.

1
  • 3
    It's not clear that streams would help you here. But you could substantially de-verbosify this by combining each pair into if (condition) { table = (ratingvalue == null) ? v1 : v2; }. Commented Dec 5, 2017 at 13:36

3 Answers 3

2

Considering that ratingvalue can only be null or non-null, you can simplify the code by writing effectively unconditional statements as such:

String processTable(String cid, String postcode, String[] ratingvalue, String ratingType) {
    if (cid != null)
        if(postcode != null)
            return ratingvalue == null? cassconf.getTable5(): cassconf.getTable6();
        else
            return ratingvalue == null? cassconf.getTable3(): cassconf.getTable4();
    if(postcode != null)
        return ratingvalue == null? cassconf.getTable1(): cassconf.getTable2();
    return "";
}

Testing the conditions with precedence first is also more efficient than testing all conditions in reversed order and overwriting results of previous evaluations.

You could also write the entire evaluation as a single condition:

String processTable(String cid, String postcode, String[] ratingvalue, String ratingType) {
    return cid != null?
        postcode != null? ratingvalue == null? cassconf.getTable5(): cassconf.getTable6():
                          ratingvalue == null? cassconf.getTable3(): cassconf.getTable4():
        postcode != null? ratingvalue == null? cassconf.getTable1(): cassconf.getTable2():
                          "";
}

An alternative is to use a map lookup:

String processTable(String cid, String postcode, String[] ratingvalue, String ratingType) {
    final int hasCID = 1, hasPostcode = 2, hasRatingValue = 4;
    Map<Integer, Supplier<String>> map = new HashMap<>();
    map.put(hasCID|hasPostcode, cassconf::getTable5);
    map.put(hasCID|hasPostcode|hasRatingValue, cassconf::getTable6);
    map.put(hasCID, cassconf::getTable3);
    map.put(hasCID|hasRatingValue, cassconf::getTable4);
    map.put(hasPostcode, cassconf::getTable2);
    map.put(hasPostcode|hasRatingValue, cassconf::getTable1);

    return map.getOrDefault(
        (cid!=null? hasCID: 0) | (postcode!=null? hasPostcode: 0)
                               | (ratingvalue!=null? hasRatingValue: 0),
        () -> "").get();
}

The key point of this alternative is that, depending on what cassconf is or when it will be initialized, the map may be prepared at an earlier stage and processTable could be simplified to the return map.getOrDefault… operation.

Sign up to request clarification or add additional context in comments.

2 Comments

cassconf will be autowired. I implemented the alternative but I'm getting a null result even though I passed a csid and postcode to the function. Trying to understand the return statement, the bitwise or '|' would ensure all the conditions are executed, right?
Correct, the bitwise or | is a numeric operation that can’t be short-circuited in general. The getOrDefault( …, () -> "") also ensures that the value returned by the map is never null. That’s also implied by the fact that .get() is invoked on that result. If the result of get() is null, then one of the getTable[1-6] methods must have returned null.
2

I was thinking this as an exercise more, nothing to do with streams though, since it can't really help you here.

You could compute a HashMap that will have O(1) time for finding a value, like so:

    Map<Integer, String> map = new HashMap<>(16);
    map.put(0b1110, "table-6");
    map.put(0b1100, "table-5");
    map.put(0b1010, "table-4");
    map.put(0b1000, "table-3");
    map.put(0b0110, "table-2");
    map.put(0b0100, "table-1");

This corresponds to whether your cid (the 4-th most significant bit), postcode (3-rd most significant bit) and ratingValue (second most significant bit) are null or not. So these are the total of 6 combinations that you are looking for.

Also this Map will have one entry per bucket, thus finding the value that you are interested in, will be really fast.

Computing the key that you need to get the value from is fairly trivial, you just need to set the bit (value that is not null).

String processTable(String cid, String postcode, String[] ratingvalue, String ratingType) {

    if (cid != null) {
        x = x | 1 << 3;
    }

    if (postCode != null) {
        x = x | 1 << 2;
    }

    if (ratingValue != null) {
        x = x | 1 << 1;
    }

    return map.get(x);
}

Do note that this code was take just as an exercise (well, we do have something close to this in real life, but there are compelling reasons for this - speed mainly).

3 Comments

I see, you had a similar idea (and were even faster), but I’d prefer named constants rather than 4th to 2nd bit. Further, you should not forget about the empty string fallback of the original code, i.e. use map.getOrDefault(x, "")… And you forgot to declare and initialize x and use different case than the parameters.
@Eugene, was just curious, what did you mean when you said "we do have something close to this in real life"?
@Swapnil I mean I do have this logic in a commercial application that I am working on a daily basis
0

Java 8 is not a magic wand you can simply wave at a piece of code to instantly improve it's readability.

Using null as a sentinel is bad practice and that's fundamentally why your code is hard to read. I suggest you rethink having nullable parameters.

Without fixing that, this is probably the best you can do:

if (ratingvalue == null)
{
    if (cid != null && postcode != null) {
        table = cassconf.getTable5();
    }
    else if (postcode != null) {
        table = cassconf.getTable1();
    }
    else if (cid != null) {
        table = cassconf.getTable3();
    }
}
else
{
    if (cid != null && postcode != null) {
        table = cassconf.getTable6();
    }
    else if (postcode != null) {
        table = cassconf.getTable2();
    }
    else if (cid != null) {
        table = cassconf.getTable4();
    }
}

3 Comments

Thanks for pointing that out, I'll correct it. Although, the best I could do is assign a dummy value to the absent parameters instead of a null as these are request parameters for an API. Excuse my ignorance, I thought there must be some concept that I was not aware of, to implement the block in java 8.
@Swapnil There are no default arguments in Java. However, similar effects can be achieved with method overloading.
My bad again, poor choice of word. I meant a dummy value such as "NA" instead of a null.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.