Vavr: We need more API.For methods

[danieldietrich]

As described here: http://blog.vavr.io/redundancy-of-the-java-language/

[CauchyPeano]

Hi @danieldietrich , link is broken, where can I find article?

[danieldietrich]

@CauchyPeano the link is fixed (the old domain name javaslang.io expired last week)

[nfekete]

@danieldietrich some of the internal links on the blog are also broken (they're absolute links pointing to javaslang.io instead of relative links). For example:

In our previous post we showed the Grill example...

[danieldietrich]

@nfekete Thank you for the hint! I will fix them!

Update: Fixed all 27 articles 😊

[CauchyPeano]

@danieldietrich following this broken links topic - there are also images links broken:
These are the results: ![for-benchmark](/content/images/2016/10/for-benchmark-1.png)

[CauchyPeano]

Hi @danieldietrich ,

Can you maybe provide some examples how it should look like?

Do you expect to have For() method on every monadic type?

[danieldietrich]

Hi @CauchyPeano,

following this broken links topic - there are also images links broken:

I fixed the image - it wasn't the link, it was a changed behavior of the markdown renderer in the the Ghost blog version.

Before I used this snippet to change the size of an image:

<div style="max-width:406px !important">
![for-benchmark](/content/images/2016/10/for-benchmark-1.png)
</div>

Now I have to use this code instead:

<img style="max-width:406px !important" alt="for-benchmark" src="/content/images/2016/10/for-benchmark-1.png">

[danieldietrich]

@CauchyPeano

Can you maybe provide some examples how it should look like?

We need to change the code generator for vavr/src-gen/main/java/io/vavr/API.java, located in vavr/generator/Generator.scala.

Wlog, currently we have:

public static <T1> For1<T1> For(Iterable<T1> ts1) { ... }

Basically, we need additional method overloads for all monadic types.

public static <T1> For1<T1> For(Option<T1> monad1) { ... }

We start with Option, Try, Either, Future and Validation. We will decide later if we do the same for all collection types (or just Seq, (Multi)Set, (Multi)Map, ... - or something else).

We will not add classes in addition to the existing API.For1, ..., API.For8. Currently we have:

    public static class For1<T1> {

        private final Iterable<T1> ts1;

        private For1(Iterable<T1> ts1) {
            this.ts1 = ts1;
        }

        public <R> Iterator<R> yield(Function<? super T1, ? extends R> f) {
            Objects.requireNonNull(f, "f is null");
            return Iterator.ofAll(ts1).map(f);
        }

        public Iterator<T1> yield() {
            return yield(Function.identity());
        }
    }

But we need to substitute Iterable with Value (and pull out the Iterator factory method call) in order to make For1..8 reusable:

    public static class For1<T1, MT1 extends Value<T1>, MR extends Value<R>> {

        private final MT1 monad1;

        private For1(MT1 monad1) {
            this.monad1 = monad1;
        }

        @SuppressWarnings("unchecked")
        public <R> MR yield(Function<? super T1, ? extends R> f) {
            Objects.requireNonNull(f, "f is null");
            return (MR) monad1.map(f);
        }

        public MT1 yield() {
            return yield(Function.identity());
        }
    }

Maybe we need to treat the generics a bit in order to make it compile...

Update: The MT1..MT8, MTR abstraction will not work for flatMap this way. Maybe we need to pass a function instead, that creates an appropriate type. I will come up later today with a working solution for the Option example.

[CauchyPeano]

thanks @danieldietrich for explanation!
I'm still not sure what to do with
public static <T1> For1<T1> For(Iterable<T1> ts1)

because there we no longer can use For1<> as result - Iterable is not a Value. Should we then remove those methods, and leave for Iterables only this one:
public static <T, U> Iterator<U> For(Iterable<T> ts, Function<T, Iterable<U>> f) ?

[danieldietrich]

@CauchyPeano For now you can leave the method signature as is and change the implementation like this:

public static <T1> For1<T1> For(Iterable<T1> ts1) {
    Objects.requireNonNull(ts1, "ts1 is null");
    return new For1<>(Iterator.ofAll(ts1));
}

You pull the Iterator.ofAll out of the For1..8 class constructor, up to the calling method.
(Iterator is part of io.vavr.collection)

Update: But we will still have problems with flatMap because it is not part of Value. I try to provide you with an update this evening...

[CauchyPeano]

Hi @danieldietrich , i'm working on it. Don't you think that it's worth to lift in signatures Iterator to Traversable then?

[danieldietrich]

@CauchyPeano Thx!

For loops iterate over Iterables. An Iterable does not need necessarily be a collection. I would stick with Iterator for now. It has a very small memory and cou footprint.

Later we might add more overloads for collections.

—-

I did not spend time on flatMap, yet. I have some ideas but need to test a bit. Will come back tomorrow...

[CauchyPeano]

@danieldietrich
Oh, spent plenty of time trying to figure out why new implementation was not working on List

   public static <T1, T2> For2<T1, T2> For(List<T1> ts1, List<T2> ts2) {
        return new For2<>(Iterator.ofAll(ts1), Iterator.ofAll(ts2));
    }

    public static class For2<T1, T2> {
        private final Iterator<T1> ts1;
        private final Iterator<T2> ts2;
        private For2(Iterator<T1> ts1, Iterator<T2> ts2) {
            this.ts1 = ts1;
            this.ts2 = ts2;
        }
        public <R> Iterator<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)));
        }
    }

then this test fails:

       final List<Integer> result = For(
            List.of(1, 2, 3),
            List.of(1, 2, 3)
        ).yield((i1, i2) -> i1 + i2).toList();

Results in
List(2,3,4)

As far as I understood it's because of mutable state of Iterator. First iterator could be consumed only once. But then I don't get why it was working before...

---

Update So yes, it's because we save iterator as field. Before we were instantiating iterator every time inside flatMap call.

[CauchyPeano]

Ok, I'm returned from vacation+new job change process so want to finish this 🤣 😢

[CauchyPeano]

@danieldietrich I was trying to recover where i've stopped last time. So right now it's not clear if we want to have flatMap on Value interface or not. From my prospective it would make sense. What do you think?

[danieldietrich]

Unfortunately flatMap does not work for Value because Java is missing higher-kinded types (aka type constructors).

So the solution should have separate For-overloads for each of our value implementations. I.e.

public static <T1> For1<T1> For(Try<T1> t1) { ... }
public static <T1, T2> For2<T1, T2> For(Try<T1> ts1, Try<T2> ts2) { ... }
// ... up to For8

public static <T1> For1<T1> For(Option<T1> t1) { ... }
public static <T1, T2> For2<T1, T2> For(Option<T1> ts1, Option<T2> ts2) { ... }
// ... up to For8

// etc

We need to utilize the Generator.scala...

[danieldietrich]

_(Copied from this discussion)_

We would need a common Monadic interface that is implemented by our 'sealed' classes (like Option, Try, List, ...). This type has to be a recursive self-type, like:

interface Monadic<M extends Monadic<M, ?>, T> {

    <U> Monadic<M, U> flatMap(Function<? super T, ? extends Monadic<? extends M, ? extends U>> mapper);

    <U> Monadic<M, U> map(Function<? super T, ? extends U> mapper);
}

I will remove the common Value interface, because it does not work well in conjunction with checked functions. E.g. Option will have flatMap(Function), Try will have flatMap(CheckedFunction).

Given that, we would also need a CheckedMonadic interface:

interface Monadic<M extends Monadic<M, ?>, T> {

    <U> Monadic<M, U> flatMap(CheckedFunction<? super T, ? extends Monadic<? extends M, ? extends U>> mapper);

    <U> Monadic<M, U> map(CheckedFunction<? super T, ? extends U> mapper);
}

Using the above interfaces, we could reduce the overall amount of For methods to (Monadic, CheckedMonadic) x (1..8) = 16 classes. This amount will stay constants when supporting more types, like Either, Future, Stream, ...

However, the flatMap API will be uglified regarding the sketched solution because the _mapper_ requires a recursive self-type (e.g. CheckedMonadic<Try<?>, U>) instead of a concrete type Try<U>. In turn, the return types of _map_ and _flatMap_ could be overloaded with concrete types.

I will defer the decision a bit if we use recursive self-types for map and flatMap (but I think, it is the right solution and it will open a whole new space for creating new APIs/better functionality).

[danieldietrich]

See https://github.com/vavr-io/vavr/pull/2281#issuecomment-410884413