Cut down API.For DSL classes to ZERO

[danieldietrich]

Background

API.For comprehensions are syntactic sugar for flatMap/map cascades.

Let f1 be a Future<T1>, f2 be a Future<T2>, f a BiFunction<T1, T2, R>.

// using flatMap/map
var res = f1.flatMap(t1 -> f2.map(t2 -> f(t1, t2)));

// using For
var res = For(f1, f2).yield(f);

Problem

Currently a For() call creates additional instances that are needed to implement our internal DSL.

On the one hand this increases the number of types on our API surface area, which is a bit artificial:

On the other hand For() leads to a class instance creation:

public final class API {

    public static <T1, T2> For2Future<T1, T2> For(Future<T1> ts1, Future<T2> ts2) {
        Objects.requireNonNull(ts1, "ts1 is null");
        Objects.requireNonNull(ts2, "ts2 is null");
        return new For2Future<>(ts1, ts2);
    }

    public static class For2Future<T1, T2> {

        private final Future<T1> ts1;
        private final Future<T2> ts2;

        private For2Future(Future<T1> ts1, Future<T2> ts2) {
            this.ts1 = ts1;
            this.ts2 = ts2;
        }

        /**
        * Yields a result for elements of the cross product of the underlying Futures.
        *
        * @param f a function that maps an element of the cross product to a result
        * @param <R> type of the resulting {@code Future} elements
        * @return an {@code Future} of mapped results
        */
        public <R> Future<R> yield(BiFunction<? super T1, ? super T2, ? extends R> f) {
            Objects.requireNonNull(f, "f is null");
            return
                ts1.flatMap(t1 ->
                ts2.map(t2 -> f.apply(t1, t2)));
        }

    }

    // ...

}

Suggested solution

If we surrender the yield word, then we could express the functionality described above by using existing functional interfaces (instead of classes):

public final class API {

    // modulo generic type bounds of the result
    public static <T1, T2, R> Function<BiFunction<T1, T2, R>, R> For(Future<T1> f1, Future<T2> f2) {
        Objects.requireNonNull(f1, "f1 is null");
        Objects.requireNonNull(f2, "f2 is null");
        return f -> f1.flatMap(t1 -> f2.map(t2 -> f(t1, t2)));
    }

}

Use-site:

var res = For(f1, f2).apply(f);

The types on the API surface area will look better then ;) I think it's worth to surrender yield.

[nfekete]

I like this idea, especially with the deprecation warning for yield.

[danieldietrich]

I will do this in the upcoming 1.0.0 in order to inform about the new reserved word 'yield'.

[pivovarit]

I will take care of this :)

[pivovarit]

I have bad news, the suggested solution won't work because of how generic parameters are resolved in Java.

A return type of a static method like this one will always end with up a resolution of R to Object regardless of what f2 type parameters are:

import io.vavr.concurrent.Future;

import java.util.function.BiFunction;
import java.util.function.Function;

class API {

    public static void main(String[] args) {
        Integer yield = For(Future.successful(42), Future.successful(42)).apply(Integer::sum); // error: incompatible types: Object cannot be converted to Integer
    }

    public static <T1, T2, R> Function<BiFunction<T1, T2, R>, R> For(Future<T1> f1, Future<T2> f2) {
        return null;
    }
}

I remember struggling with similar issues when trying to develop a user-friendly API for parallel-collectors.

To be honest, the current API looks like a boilerplate-heavy solution to the above problem (and it works just fine since there's a limited number of custom types that are supported) and doesn't look that bad anymore.

Luckily, since we're targeting 2.0.0 with a revised API, we're not in a rush.

[thadumi]

This is because during the type inference of For there isn't a type R provided. Therefore for Java, your code is like
public static <T1, T2> Function<BiFunction<T1, T2, ?>, ?> For(Future<T1> f1, Future<T2> f2)

Anyway, in your example, the return type is not an Integer but a Future<Integer>. Therefore, the actual signature should be

public static <T1, T2> Function<BiFunction<T1, T2, ?>, Future<?>> For(Future<T1> f1, Future<T2> f2)

at this point, you could cast from Future<?> to Future<R>. (of course, this leads to the warning issue for casting from '?')

Another, but ugly solution, could be adding the return type as a parameter.

    public static void main(String[] args) {
        Future<Integer> yield = For(Future.successful(42), Future.successful(42), Integer.TYPE).apply(Integer::sum);
    }

    public static <T1, T2, R> Function<BiFunction<T1, T2, R>, Future<R>> For(Future<T1> f1, Future<T2> f2, Class<R> type) {
        return f -> f1.flatMap(t1 -> f2.map(t2 -> f.apply(t1, t2)));
    }

[nfekete]

@thadumi You won't be able to express generic types with passing class literals (only with very ugly double casting).

[danieldietrich]

@pivovarit @thadumi @nfekete That's right. We would need to fall back to the old 'builder-pattern' solution, by adding an intermediate class-type returned by For. But I don't wan't to do that, it blows up our API with unnecessary types.

Another solution would be to omit the 'apply' syntax. This would be even faster, because we would save one lambda instance.

But we always have another solution as well: if in doubt about an API, just leave it away. In our case it is syntactic sugar for flatMap/map.

What do you think? Would it be a benefit for our users to have the For-syntax?

[nfekete]

I think the For construct is much nicer than using nested flatMap/maps, especially with more parameters. I would keep the construct in one way or another.

[larousso]

Hi, I have the feeling that this is a zip method that could be in Either, Option, Future...

Either.zip(either1, either2, (r1, r2) -> r3);
Either.zip(either1, either2, either3, (r1, r2, r3) -> r4);

and so on ...

[danieldietrich]

@larousso Thank you for sharing your thoughts! The goal of this issue is to reduce the number of overloads of the methods. You suggestion would introduce N versions of .zip x number of types (Option, Try, ...), where N is the max function arity. My current strategy is to reduce the number of such API overloads. I will do this not only in API.java but also in Try, there we have several Try.withResources variants. We will have practicable alternatives.

[larousso]

OK I understand your point but as a end user I find it conveniant to have the list of methods I can use using ctrl space and find what I can use using the signature of the methods.

[eslep]

@danieldietrich @nfekete I also really enjoy having the For within Vavr, and dropping it in favor of chaining flatMap/map is something I could see giving the impression (to some people/newbies) that "functional programming is messy and complex". For a person who is not yet so familiar with the concepts, it's much simpler to explain 'this calls function f if all of these Options are defined' than to show them a chain of flatMap/map.

So I see it not only as helpful syntactic sugar, but also as something which might make code a bit more palatable to those who are still becoming familiar with the concepts.

I don't know if you've put any work into this yet, but I'm happy to make a pull request since it's quite a simple change to the generator. (Unless there is more to do which I am overlooking?) I definitely would prefer the proposal you made, and I think that keeping the For naming also keeps the 'follow Scala' mindset more than the zip suggestion.

There is only one 'conflict' needing to be addressed with this approach, the first two methods for Iterable have unfortunately the same erasure.

I see a few options for resolving this:

1. Rename the first method which supports the nested syntax a. Unfortunately, slightly less intuitive than simply having the For naming universally and also can't 'follow Scala'
2. Drop support for the nested syntax a. Is a clean option, but drops this Scala feature altogether and unfortunately breaks any code which relied on the nested semantics... would probably need to be replaced by something like flatMap/map by anyone relying on the nested semantics
3. Keep the class-based approach for only the Iterable case a. Is not awesome... and probably a bit confusing for people who would wonder why the Iterable implementation differs from the others... but it does resolve the issue of the type signatures conflicting

I don't know if you have any other/better ideas which I've overlooked? Personally, I think I am leaning towards option 1... but none of them are completely awesome 🤷