Vavr: Opportunity to Improve Conversion of Validation.Invalid<? extends Throwable,T> to Try<T>

[Bill]

I have a Validation and I'd like it to cause an exception carrying the Validation.Error.error content, somehow.

My first thought was to invoke mapError() on my Validation to roll the error content into a Throwable, then simply to invoke toTry() on the new Validation. Unfortunately that invokes Value.toTry() which attempts a get() even if the Validation is an Error. There is no specialized behavior that somehow notices that the Error is actually carrying a Throwable, so the error _could_ be used to initialize the cause in a new Try.Failure.

My second thought was to invoke getOrElseThrow() directly on my original Validation. Unfortunately, that method does not provide visibility to the Validation.Error.error content. That method takes a Supplier<X>.

As a workaround, I mapError() on my Validation to transform my (String) error content into a Throwable (carrying the String in the message). Then I wrote my own method to convert such a Validation into a Try:

    private static <T> Try<T> toTry(final Validation<? extends Throwable,T> v) {
        // avoid calling v.toTry() if v.isInvalid()
        return v.isValid() ? v.toTry() : Try.failure(v.getError());
    }

I see various possible solutions that would obviate my approach (calling mapError() followed by my toTry() method above). :

1. Make Validation.Invalid.toTry() construct a Try.Failure:
   a. …by making Value.toTry() aware of the possibility that the "alternative" (in this case Validation.Invalid.error) might be a valid argument to the Try.failure() constructor. In that case, instead of calling get(), it could call Try.failure().
   b. …or by defining a Validation.Invalid.toTry() that constructs the Try.Failure if the cause is Throwable.
2. Overload (or redefine) Validation.getOrElseThrow() to take a Function1<? extends Throwable,T>. Then getOrElseThrow could pass the Validation.Error.content (in this case, a Throwable) to the fn.

Update: as I learned later (see my next message) (2) would bring Validation.getOrElseThrow() into alignment with Either.getOrElseThrow() and so it might be a minimally-disruptive change. That being said, it seems to me that if (1) could be made to work, the approach should be applied to Either as well as Validation. If toTry() on a thing carrying a Throwable in its "alternative" created a Try with that Throwable as the error, toTry() would be less surprising to my way of thinking.

Here's a test (vavr 0.9.2, junit 4.12):

import org.junit.Before;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ErrorCollector;

import io.vavr.control.Try;
import io.vavr.control.Validation;

import static org.junit.Assert.*;
import static org.hamcrest.CoreMatchers.*;

public class ValidationToTryTest {

    private final Validation<String,String> invalid =
            Validation.invalid("a problem");

    private final Validation<? extends Throwable,String> invalidWithThrowable =
            invalid.mapError(IllegalArgumentException::new);

    @Rule
    public ErrorCollector collector = new ErrorCollector();

    @Test
    public void testConversionViaMapErrorThenToTry() {
        final Try<String> tryGet = Try.of(()-> invalidWithThrowable.toTry().get());
        collector.checkThat(tryGet.isFailure(), is(true)); // ok
        /*
         fails:
         Expected: is an instance of java.lang.IllegalArgumentException
         but: <java.util.NoSuchElementException: get of 'invalid' Validation> is a java.util.NoSuchElementException
         */
        collector.checkThat("Validation.toTry() doesn't have a specialization for Validation<? extends Throwable,T>",
                            tryGet.getCause(),is(instanceOf(IllegalArgumentException.class)));
    }

    @Test
    public void testConversionViaGetOrElseThrow() {
        final Try<String> tryGet = Try.of(()-> invalid.getOrElseThrow(IllegalArgumentException::new));
        collector.checkThat(tryGet.getCause(),is(instanceOf(IllegalArgumentException.class))); // ok
        /*
         fails:
         Expected: is "a problem"
         but: was null
         */
        collector.checkThat("getOrElseThrow() doesn't have visibility to the Invalid's error",
                            tryGet.getCause().getMessage(),is("a problem"));
    }

    @Test
    public void testConversionViaMapErrorThenUserDefinedToTry() {
        final Try<String> tryGet = Try.of(()-> toTry(invalidWithThrowable).get());
        collector.checkThat(tryGet.isFailure(), is(true)); // ok
        collector.checkThat(tryGet.getCause(),is(instanceOf(IllegalArgumentException.class))); // ok
    }

    private static <T> Try<T> toTry(final Validation<? extends Throwable,T> v) {
        // avoid calling v.toTry() if v.isInvalid()
        return v.isValid() ? v.toTry() : Try.failure(v.getError());
    }

}

[Bill]

A colleague figured out that we can first Validation.toEither() and then use getOrElseThrow() (on the Either.Left or Either.Right). The latter methods (unlike getOrElseThrow() on a Validation, inherited from Value) happen to take a Function<? super L, X> and that fn gets passed the optional content (in this case the Left content).

In this light, it seems like an oversight, that there is no getOrElseThrow(Function<? super L, X>) on Validation.Valid/Validation.Invalid to match the corresponding methods on Either.Left/Either.Right.

Here is an updated JUnit 4 test that exercises all four approaches, testing all three conditions on each:

Whole project: https://github.com/Bill/validation-to-try

The test:

import java.util.Arrays;
import java.util.Collection;

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ErrorCollector;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import io.vavr.control.Either;
import io.vavr.control.Try;
import io.vavr.control.Validation;

import static org.hamcrest.CoreMatchers.*;

@RunWith(Parameterized.class)
public class ValidationToTryTest {

    @Parameterized.Parameters
    public static Collection<Object[]> data() {
        final Validation<String,String> invalid =
                Validation.invalid("a problem");

        final Validation<? extends Throwable,String> invalidViaMapError =
                invalid.mapError(IllegalArgumentException::new);

        final Either<String,String> leftEither =
                invalid.toEither();

        return Arrays.asList(new Object[][] {

                { "via mapError() then toTry()",
                  Try.of(()-> invalidViaMapError.toTry().get()) },

                { "via getOrElseThrow()",
                  Try.of(()-> invalid.getOrElseThrow(IllegalArgumentException::new)) },

                { "via mapError() then user-defined toTry()",
                  Try.of(()-> toTry(invalidViaMapError).get()) },

                { "via toEither() then toTry()",
                  /*
                   If we try to use a method reference here, the compiler helpfully informs us that the
                   reference is ambiguous. You see, IllegalArgumentException has a default constructor
                   and one that takes a String. getOrElseThrow() is defined up in Value and down in
                   Either. The one up in Value takes a Supplier whereas the one down in Either takes
                   a Function. The latter is passed the left value if the Either isLeft()
                   */
                  Try.of(()-> leftEither.getOrElseThrow(error->new IllegalArgumentException(error)))}

        });
    }

    @Parameterized.Parameter
    public String description;

    @Parameterized.Parameter(1)
    public Try<String> tryGet;

    @Rule
    public ErrorCollector collector = new ErrorCollector();

    @Test
    public void test() {
        collector.checkThat(description, tryGet.isFailure(), is(true));
        collector.checkThat(description, tryGet.getCause(),is(instanceOf(IllegalArgumentException.class)));
        collector.checkThat(description, tryGet.getCause().getMessage(),is("a problem"));
    }

    private static <T> Try<T> toTry(final Validation<? extends Throwable,T> v) {
        // avoid calling v.toTry() if v.isInvalid()
        return v.isValid() ? v.toTry() : Try.failure(v.getError());
    }

}

The first scenario (via mapError() then toTry()) fails two conditions:

java.lang.AssertionError: via mapError() then toTry()
Expected: is an instance of java.lang.IllegalArgumentException
     but: <java.util.NoSuchElementException: get of 'invalid' Validation> is a java.util.NoSuchElementException

java.lang.AssertionError: via mapError() then toTry()
Expected: is "a problem"
     but: was "get of 'invalid' Validation"
Expected :a problem
Actual   :get of 'invalid' Validation

The second scenario (via getOrElseThrow()) fails one condition:

java.lang.AssertionError: via getOrElseThrow()
Expected: is "a problem"
     but: was null

The last two scenarios pass all tests.

[Bill]

Back in January (above) I came to the conclusion that, it seemed like an oversight, that there was no getOrElseThrow(Function<? super L, X>) on Validation.Valid/Validation.Invalid to match the corresponding methods on Either.Left/Either.Right that take Functions.

Yesterday I ran into an analogous problem trying to convert a Try to a Validation:

Validation<String,Apple> doSomeValidation(final String name) {
    return Try.withResources(() -> new AppleReader(name)).of(reader->{
      // do things with the reader. some things cause exceptions
      return reader.read();
    }).toValidation( /* I'd like to convert the Throwable into a Validation error here! */ );
}

But the Try does not send its error (Throwable) to the function supplied by the caller, so I had to do this:

Validation<String,Apple> doSomeValidation(final String name) {
    final Try<Apple> apple = Try.withResources(() -> new AppleReader(name)).of(reader->{
      // do things with the reader. some things cause exceptions
      return reader.read();
    });
    return apple.toValidation(()->apple.getCause().toString());
}

Notice the yucky introduction of an extra statement to capture the intermediate variable.

Try has:

    default <L> Validation<L, T> toValidation(L invalid)
    default <L> Validation<L, T> toValidation(Supplier<? extends L> invalidSupplier)

Both inherited from Value. But there is no conversion to Validation that has access to the error if the Validation isInvalid(). That is, there is no variant of this function that takes a Function/Function1.

So we have a bunch of types that model "alternatives". All but one (Option) carry important information in the "left"/"cause"/"error" part:

Either
Try
Validation

For each of those, there are various higher-order functions that should, in general, pass the "left"/"cause"/"error" to the caller-supplied function, but do not.

Searching through Value, Either, Try, and Validation for "supplier" I gathered the full list of functions of interest. On each of Either, Try, and Validation I think these functions should be overloaded to take a Function/Function1:

// unless otherwise noted, "left"/"cause"/"error" value will be passed to fn

Try.       toEither(fn)
Validation.toEither(fn)
Either.    toTry(fn)
Validation.toTry(fn)
Either.    toValidation(fn)
Try.       toValidation(fn)


// each of the following should be defined on each of Try, Validation, and Either:

getOrElse(fn)
getOrElseThrow(fn)
getOrElseTry(fn)

orElse(fn)

toInvalid(fn)    // pass "right"/"value" value to fn
toValid(fn)      // pass "left"/"cause"/"error" value to fn
toLeft(fn)       // pass "right"/"value" value to fn
toRight(fn)      // pass "left"/"cause"/"error" value to fn

With that done, my validation method could look something like this:

Validation<String,Apple> doSomeValidation(final String name) {
    return Try.withResources(() -> new AppleReader(name)).of(reader->{
      // do things with the reader. some things cause exceptions
      return reader.read();
    }).toValidation( throwable->throwable.toString() );
}

[danieldietrich]

Hi @Bill, thank you for putting so much effort into this issue!

I fully agree with your suggested list of overloads! We should definitely add those.

• Try.toValidation(fn) is already there on our master branch.
• we prefer the more general Function over Function1 as argument type

[CauchyPeano]

What we have seen so far:

Validation:
Either<Seq<E>, T> toEither()
Validation<E, T> fromEither(Either<E, T> either)
Validation<Throwable, T> fromTry(Try<T> t)

Either:
Validation<L, R> toValidation()

Try:
Either<Throwable, T> toEither()
Option<T> toOption()
Optional<T> toOptional()
Validation<Throwable, T> toValidation()
Validation<U, T> toValidation(Function<Throwable, U> throwableMapper)

Option:
Optional<T> toOptional()
Try<T> toTry()
Try<T> toTry(Supplier<Throwable> ifEmpty)

Firstly I see that we have both ways to convert between Validation<->Try and Validation<->Either.
From all discussions I suggest to keep only to* conversions methods and remove from* methods.

[Bill]

Generally, since Java methods can be overloaded on argument type but not on return type, fromXXX() methods don't seem to make much sense in a Java library.

Is that what you are getting at @CauchyPeano?

[CauchyPeano]

@Bill, I agree, but I want to say that it's too much to have duplicated conversion methods on the source type (e.g. Validation.fromEither(sourceEither)) and same conversion on target type (e.g. sourceEither.toValidation()). It's worth to pick something one to make API consistent.

@danieldietrich I've read that you want to minify current API footprint. Can you please confirm once more which methods you want to have and which ones not.

[Bill]

@CauchyPeano I don't think I can say it any more clearly than I did on March 8.

[danieldietrich]

Sadly, I have to admit that lately I have been a little careless in the care of the issues. With Vavr 1.0.0, we align more tightly to Scala. I've finished a first alpha version of the vavr-control module, which will contain Option, Either and Try. Especially, Validtation will reside outside of that module.

The conversion methods of the vavr-control module have (more or less) a final state now. So this issue is obsolete. I will close it.

---

@CauchyPeano this also has an impact on your PR #2304. I really appreciate your effort, sorry for discarding your work.