Vavr: orElse abstraction on Option<T> not behaving as expected

[Vivek-Patil]

return Seq(Tuple.of(currTxn, maybeLatestTxn))
                .flatMap(tuple ->
                        checkNew.apply(c1, c2, s1, s2, tuple._1, tuple._2)
                                .orElse(checkDowngrade.apply(c1, c2, s1, s2, tuple._1, tuple._2)
                                .orElse(checkUpgrade.apply(c1, c2, s1, s2, tuple._1, tuple._2))
                                .orElse(checkRenew.apply(c1, c2, s1, s2, tuple._1, tuple._2))
                                .orElse(checkExpire.apply(c1, c2, s1, s2, tuple._1, tuple._2))
                                )
                ).toTry();

private Function6<Integer, Option<Integer>, String, Option<String>,
            TransactionData, Option<TransactionData>, Option<TransactionData>> checkNew =
            (c1, c2, s1, s2, tx1, tx2) -> Seq(tx1)
                    .filter(t -> tx2.equals(Option.none()))
                    .map(t -> t.setEvent(DBEvent.CREATE))
                    .toOption();

default Option<T> orElse(Option<? extends T> other) {
        Objects.requireNonNull(other, "other is null");
        return isEmpty() ? (Option<T>) other : this;
    }

@SuppressWarnings("unchecked")
   default Option<T> orElse(Supplier<? extends Option<? extends T>> supplier) {
        Objects.requireNonNull(supplier, "supplier is null");
        return isEmpty() ? (Option<T>) supplier.get() : this;
    }

I have this code where I am kind of applying functions in a chain operation if the output of the previous func is None where I am using a orElse abstraction to achieve this, but the problem I am facing is even if I get the output of the first func checkNew as Option<TransactionData> its still evaluating the next function checkDowngrade and it breaks my code as I am not expecting a None somewhere in the func evaluation which should ideally not happen.

Digging further I found out that there are 2 abstractions of orElse available in the type which i have mentioned above in the code block.
I noticed with a dummy example that If I use the second one which take a supplier it works as expected where it would not execute further if the o/p of the previous if None. But somehow when I am using the library it automatically uses the first abstraction of orElse which faces the problem I stated above. Is there something that I am missing here ?

@danieldietrich

[danieldietrich]

Thank you for your question. I took me some time to get your example compiling because there are missing many type informations. For the next time it would help me if you could provide a more _minimal_ and _complete_ example.

This is what I've come up with:

import javaslang.control.Option;
import javaslang.control.Try;

import static javaslang.API.*;

public class Test {

    Try<TransactionData> test(int c1, Option<Integer> c2, String s1, Option<String> s2, TransactionData currTxn, Option<TransactionData> maybeLatestTxn) {
        return Seq(Tuple.of(currTxn, maybeLatestTxn))
                .flatMap(tuple ->
                        checkNew.apply(c1, c2, s1, s2, tuple._1, tuple._2)
                                .orElse(checkDowngrade.apply(c1, c2, s1, s2, tuple._1, tuple._2))
                                .orElse(checkUpgrade.apply(c1, c2, s1, s2, tuple._1, tuple._2))
                                .orElse(checkRenew.apply(c1, c2, s1, s2, tuple._1, tuple._2))
                                .orElse(checkExpire.apply(c1, c2, s1, s2, tuple._1, tuple._2))
                ).toTry();
    }

    private Function6<Integer, Option<Integer>, String, Option<String>,
            TransactionData, Option<TransactionData>, Option<TransactionData>> checkNew =
            (c1, c2, s1, s2, tx1, tx2) -> Seq(tx1)
                        .filter(t -> tx2.equals(Option.none()))
                        .map(t -> t.setEvent(DBEvent.CREATE))
                        .toOption();

    private Function6<Integer, Option<Integer>, String, Option<String>,
            TransactionData, Option<TransactionData>, Option<TransactionData>> checkDowngrade = null;

    private Function6<Integer, Option<Integer>, String, Option<String>,
            TransactionData, Option<TransactionData>, Option<TransactionData>> checkUpgrade = null;

    private Function6<Integer, Option<Integer>, String, Option<String>,
            TransactionData, Option<TransactionData>, Option<TransactionData>> checkRenew = null;

    private Function6<Integer, Option<Integer>, String, Option<String>,
            TransactionData, Option<TransactionData>, Option<TransactionData>> checkExpire = null;

    interface TransactionData {
        TransactionData setEvent(DBEvent dbEvent);
    }

    enum DBEvent {
        CREATE
    }

}

Because it does compile, the types are right. Is this what your types look like, especially for the functions I added?

I understand that there are two types of orElse signatures. But I can't see a compiler conflict here.

Which IDE/compiler do you use? Eclipse/ECJ or IntelliJ IDEA/javac?

Thx

- Daniel

[danieldietrich]

Which version of Javaslang do you use?

[danieldietrich]

I think you use 2.1.0-alpha because of the API.Seq(...) factory method.

I highly recommend to use the latest stable bugfix release (currently 2.0.5).

Your code can be simplified like this:

    return Option.of(Tuple.of(currTxn, maybeLatestTxn))
            .flatMap(tuple ->
                    checkNew.apply(c1, c2, s1, s2, tuple._1, tuple._2)
                            .orElse(checkDowngrade.apply(c1, c2, s1, s2, tuple._1, tuple._2))
                            .orElse(checkUpgrade.apply(c1, c2, s1, s2, tuple._1, tuple._2))
                            .orElse(checkRenew.apply(c1, c2, s1, s2, tuple._1, tuple._2))
                            .orElse(checkExpire.apply(c1, c2, s1, s2, tuple._1, tuple._2))
            ).toTry(() -> new Error("Can't create TransactionData"));

I used Option instead of Seq and toTry instead of toOption. Given that you are able to provide a meaningful error message (instead of NoSuchElementException(...)).

[danieldietrich]

Regarding your initial question: You have to use Suppliers with orElse in order to ensure that check* aren't evaluated if not necessary(!), i.e.

    return Option.of(Tuple.of(currTxn, maybeLatestTxn))
            .flatMap(tuple ->
                    checkNew.apply(c1, c2, s1, s2, tuple._1, tuple._2)
                            .orElse(() -> checkDowngrade.apply(c1, c2, s1, s2, tuple._1, tuple._2))
                            .orElse(() -> checkUpgrade.apply(c1, c2, s1, s2, tuple._1, tuple._2))
                            .orElse(() -> checkRenew.apply(c1, c2, s1, s2, tuple._1, tuple._2))
                            .orElse(() -> checkExpire.apply(c1, c2, s1, s2, tuple._1, tuple._2))
            ).toTry(() -> new Error("Can't create TransactionData"));

I hope this helps.

I'm pretty sure that the current Option methods work, there is no error. The functionality is well covered by unit tests.

If you don't mind, I will close the issue.

[Vivek-Patil]

@danieldietrich I am using the 2.1.0-alpha as you said because of the await() functionality on the Future<T> which I needed the most in my code. I am using Intellij Idea 2017 as my IDE. If I switch to the 2.0.5 I have to replace my code where await() on Future<T> does not return Future<T>.I know there is a workaround for it but just wanted that functionality so used the latest version. What do you suggest ?

[danieldietrich]

@Vivek-Patil If you do not have any transitive dependencies that depend on Javaslang, you may use 2.1.0-alpha. Currently I'm preparing 2.1.0-beta.

I also use IntelliJ IDEA 2017. The code I've written above works with 2.0.5 and 2.1.0-alpha. Please use orElse with Suppliers and all should be fine.

[Vivek-Patil]

@danieldietrich Also why is it such that I have to replace toOption() with toTry() ? It should also work on even if I return atoOption(), but it takes me to theorElse()` with one not using the supplier. Is it something because of the javaslang latest alpha version ?

[danieldietrich]

No, toOption() is perfectly fine. But I replaced Seq with Option at the beginning. I see no reason to use a collection. The natural use-case is to use an Option directly. However, it is your choice.

I will close this issue because there is no reason to change the code-base. Feel free to ask more question in this issue if still something isn't clear to you.

[Vivek-Patil]

@danieldietrich Got it. You replaced the function provided in the orElse block to a explicit function argument which I did not notice as it takes a supplier. Thanks for your immediate help.

Thanks
Vivek