Improve onItem/onFailure produceX method

[cescoffier]

This PR provides better methods to chain async calls. This topic has been discussed in many issues, so here is my take on it.

The main idea is to use applyAsync for method returning Uni and generate for method returning Multi:

• Uni.onItem().produceUni() -> Uni.onItem().applyAsync()
• Uni.onItem().produceMulti() -> Uni.onItem().stream()
• Multi.onItem().produceUni() -> Multi.onItem().applyAsync() / Multi.onItem().applyAsyncAndConcatenate() / Multi.onItem().applyAsyncAndMerge()
• Multi.onItem().produceMulti() -> Multi.onItem().stream() / Multi.onItem().streamAndConcatenate() / Multi.onItem().streamAndMerge()

Multi's produceIterable became streamFromIterable.
produceCompletionStage became applyCompletionStage.

These names have been chosen conscientiously. I discarded many alternatives after having tried them and compare the pros and cons of each alternative. Typically chain (one popular option) did not fit on multi. Some alternatives were losing the event-driven mantra. Some shorter approach ended up mixing concepts or didn't express the intent correctly.

The PR also adds invokeAsync to fix #151.

Update: generate was not great, I changed it to stream CC @kenfinnigan

[cescoffier]

@jponge as discussed.

[gavinking]

I'm not really seeing how this solves the problem. .onItem().applyAsync() is still a way too verbose and messy-lookin' way to chain Unis. So I'm going to have to keep using flatMap() for this.

I can't even see how the term "async" crept in there. Why would you not have called it applyUni() which is shorter and uses words that are already part of your API vocabulary instead of introducing a new term?

In summary I don't see how these proposed changes solve the problem. I'm not even convinced that it's an improvement on the status quo.

[cescoffier]

@gavinking yes, it's verbose, but the flatten approach is terrible. I ended up with 380 methods in Multi which is a no go. So, yes, there is still some work to do on the naming, and this PR is far from being finalized.

I tried with chain - it didn't work. For multi it does not work.

Alternatives are welcome.

[gavinking]

I ended up with 380 methods in Multi which is a no go.

Then let's discuss it. The rest of us don't even have a clear idea of the requirements and design constraints unless you write them down.

I tried with chain - it didn't work. For multi it does not work.

But as I understand it (I may not), the current PR uses generate() for Multi. So what was wrong with chain() + generate().

[cescoffier]

I ended up with 380 methods in Multi which is a no go.

Then let's discuss it. The rest of us don't even have a clear idea of the requirements and design constraints unless you write them down.

Definitely.

One of the main ideas of Mutiny is to avoid the flat design we are seeing in other libraries which leads to "let's use the IDE completion to pick the right operator without understanding it". That's what produced this idea of onX groups and limit the size of the API. That's why I would like to limit the number of methods in the main classes. I don't have a strict magic number, but I would prefer 30-50 to 100+.

I'm ok with having shortcuts. We already have some (map, flatMap, concatMap). As illustrated in the other PR (#169), adding a peek like operation is on its way (just a naming thing ;-)). Where I don't believe it's good is to flatten everything. Shortcuts should be motivated.

I tried with chain - it didn't work. For multi it does not work.

But as I understand it (I may not), the current PR uses generate() for Multi. So what was wrong with chain() + generate().

Let's try to summarize.
You receive an item from a Uni. What can you do (with the current method and the proposed method):

• invoke a callback that does not modify the item (invoke)
• invoke a callback executing an async action but once done, continue with the original item.
• modify the item (apply)
• produce a Uni (produceUni -> applyAsync )
• produce a Multi (produceMulti -> stream (was generate ))

On a multi, it's a bit more tricky, because you have a stream of items. So, when you receive an item, you can:

• [same as Uni] invoke a callback that does not modify the item (invoke)
• [same as Uni] invoke a callback executing an async action but once done, continue with the original item.
• [same as Uni] modify the item (apply)
  // Here the fun begins
• produce a Uni (produceUni -> applyAsync ). Except that you need to indicate how the produced items are flattened. So, applyAsyncAndConcatenate and applyAsyncAndMerge
• produce a Multi (produceMulti -> stream (was generate )). Same issue here, you need to indicate how the items are flattened.

[gavinking]

One of the main ideas of Mutiny is to avoid the flat design we are seeing in other libraries which leads to "let's use the IDE completion to pick the right operator without understanding it". That's what produced this idea of onX groups and limit the size of the API. That's why I would like to limit the number of methods in the main classes

Yes, I understand, and I support that goal.

But, and this is a hug caveat: I don't think it's much better to have three interfaces with 100 operations each than it is to have one interface with 300 operations. It might even be worse.

So what I've been trying to say is I want you to write down what you think are the fundamental, basic composable operations that your API needs to be built up from.

I'm ok with having shortcuts.

But I'm explicitly advocating fewer shortcuts. It seems to me that Mutiny as it exists today is sacrificing usability/understandability of the basic/fundamental operations, in order to provide a bunch of extra "convenience" or "advanced" methods that I don't feel like I'm ever going to need to use. But I might very well be wrong, because I don't have a clear idea of all the usecases!

I think it's somewhat of a design smell to have various ways to access the same operation, e.g. flatMap() vs. onItem().produceUni(). And it's even a bit confusing. (Well, it's not that confusing, but it does make me doubt that I'm "using it as intended".)

Let's try to summarize.

Great, but I want to see a summary like that which also includes all the basic operations needed for e.g. error handling, null handling, etc, as you see them.

[gavinking]

• invoke a callback that does not modify the item (invoke)
• invoke a callback executing an async action but once done, continue with the original item.
• modify the item (apply)
• produce a Uni (produceUni -> applyAsync )
• produce a Multi (produceMulti -> stream (was generate ))

But even this characterization misses something I think.

There's the distinction between:

• apply() vs
• invoke()

And there's also a distinction between:

• I have a plan function, and
• I have a Uni

And these cases are orthogonal. It seems to me that the list above misses the need for invokeUni(), which if I'm not mistaken, was already requested in issues #151, #155, and #83.

In general, any operation you have which is going to accept a function, is also going to need a variation which accepts a Uni.

So as a first step, that requirement needs to be made clear as a design constraint.

[cescoffier]

But, and this is a huge caveat: I don't think it's much better to have three interfaces with 100 operations each than it is to have one interface with 300 operations. It might even be worse.

It's not the case. We are far from 100 methods in each group. It gives you a navigation structure.

I think it's somewhat of a design smell to have various ways to access the same operation, e.g. flatMap() vs. onItem().produceUni(). And it's even a bit confusing. (Well, it's not that confusing, but it does make me doubt that I'm "using it as intended".)

That was the first feedback I had: where are the map/flatMap operations. It's the basic operations from popular reactive libraries. I, personally, don't like flatMap (I've nothing against map). flatMap is ambiguous and the behavior depends on the object you are manipulating: uni.flatMap != multi.flatMap.

And these cases are orthogonal. It seems to me that the list above misses the need for invokeUni(), which if I'm not mistaken, was already requested in issues #151, #155, and #83.

That has been provided (as invokeAsync in this PR).

[gavinking]

• produce a Uni (produceUni -> applyAsync ). Except that you need to indicate how the produced items are flattened. So, applyAsyncAndConcatenate and applyAsyncAndMerge
• produce a Multi (produceMulti -> stream (was generate )). Same issue here, you need to indicate how the items are flattened.

And you need two variations of each of these operations, one accepting a plain function, and one accepting a Uni, no?

[cescoffier]

Sorry missed that part:

Great, but I want to see a summary like that which also includes all the basic operations needed for e.g. error handling, null handling, etc, as you see them.

Basically, whatever event you receive that's going to be one of these methods. Sometimes slightly differently, as for example, when you receive the completion event, you don't have an item.

[gavinking]

That was the first feedback I had: where are the map/flatMap operations. It's the basic operations from popular reactive libraries. I, personally, don't like flatMap (I've nothing against map). flatMap is ambiguous and the behavior depends on the object you are manipulating: uni.flatMap != multi.flatMap.

Right, I agree, I'm not in love with the name flatMap() either! We don't have a Functor abstraction here, so calling this operations flatMap() is just wannabe functional programmers being pretentious.

But no way in hell am I going to have hundreds of instances of .onItem().produceUni() scattered throughout my entire program. It's obnoxious!

[cescoffier]

• produce a Uni (produceUni -> applyAsync ). Except that you need to indicate how the produced items are flattened. So, applyAsyncAndConcatenate and applyAsyncAndMerge
• produce a Multi (produceMulti -> stream (was generate )). Same issue here, you need to indicate how the items are flattened.

And you need two variations of each of these operations, one accepting a plain function, and one accepting a Uni, no?

The flattening part is only relevant when producing asynchronous items. A plain synchronous function would preserve the order.

For example, let's have a multi: {a, b, c}.

• multi.onItem().apply(letter -> letter.toUppercase()) would produce {A, B, C}
• multi.onItem().applyAsync(letter -> doSomethingAsync(letter)) would produce 3 Unis (one for each item). However, you don't know when these uni will fire their item. If you want to preserve the order, you need to use applyAsyncAndConcatenate(). If you accept interleaved items, andMerge().
• multi.onItem().stream(l -> getAStream(l)) has the same behavior. You need to decide how these streams are flattened.

[cescoffier]

Sorry missed that part:

Great, but I want to see a summary like that which also includes all the basic operations needed for e.g. error handling, null handling, etc, as you see them.

Basically, whatever event you receive that's going to be one of these methods. Sometimes slightly differently, as for example, when you receive the completion event, you don't have an item.

Forgot one interesting case: the absence of an event ( ~ a timeout).

[gavinking]

@cescoffier These are the "basic operations" as I understand them:

1. transform the current item, affecting all subsequent operations on the stream
2. do something with the current item, without affecting the rest of the stream
3. handle a failure, replacing the failure with an item
4. do something with the current item and the current failure, if any, without affecting the rest of the stream

Any of the above operations must accept either a plain (synchronous) function, or an asynchronous function packaged as a Uni.

In addition, there are operations for working with Multis, that I don't have a really clear notion about because I just have not come across a really convincing use for Multi yet.

But I know that this list isn't exhaustive because there's a whole bunch more functionality in Mutiny which isn't captured in the above list. And 2*8 = 16 operations isn't coming anywhere close to 350 methods. So I would dearly like to know where the additional >300 methods are coming from and what they are for.

[FroMage]

To recap, Mutiny was born mostly to solve the issue of RxJava having grown to use more than two types (Completable, and Maybe were a pain), and while we were thinking about it, we all agreed that a second possibly bigger reason for existence and selling point could be usability, because IDE completion was unusable and names were too vague and competing.

I was initially very unsure about the idea, but after having used it, I find the concept MUCH friendlier than the flat design, and much more intuitive.

Now, it's necessarily more verbose, which is precisely why the most common opertions flatMap and map have shortcuts. We could debate adding shortcuts for other operations we observe to be super common in practice, but I doubt they come near the frequency of map/flatMap to justify the exception.

Now, as to this PR, I don't quite understand why applyAsync is better than produceUni as a name, and similarly for stream versus produceMulti. I don't see the justification for the name change, and I find the new names less good because they're not related, and Async implies behaviour that is not actually related.

If produceUni turned into applyAsync to make it more related to apply, then I think that's the wrong reason or the wrong suffix.

IMO we could drop the parallelism of produceUni/produceMulti just because one of the two is going to be a conversion to a different type. From Uni to Multi, for example. While the other is going to turn a Uni into another Uni, so they're really fundamentally different objectives, IMO.

While apply/produceUni are indeed related, in the same way that map/flatMap are related, except that's only on the surface, because apply keeps the current chain while produceUni really switches the current Uni for another source entirely. In practice it doesn't matter much, but you could also name produceUni substitute or replace or switcharoo because that's what it does.

On the other hand, keeping the two names related could be good for discovery, but then I'd rather get apply and applyUni.

As for the Uni/Multi conversion, we could keep them all related with apply/applyUni/applyMulti or separate them and move produceMulti to Uni.convert().toMulti() for all I care. Most of the times I have to convert a Multi<T> into a Uni<List<T>> or the opposite and I have to use other methods than those anyways.

All in all, I don't find the new names much better if at all. I also think they miss the point that @gavinking was making by email, which is an entirely different strategy altogether, and which would perpahs be easier to discuss if @gavinking himself opened a PR with the API proposal so we can discuss it while trying it.

[gavinking]

The flattening part is only relevant when producing asynchronous items. A plain synchronous function would preserve the order.

OK, so the relationship between Multi and ordering is something I don't quite understand. Are you saying that Multis are really intended to represent unordered streams?

The semantics of Multi isn't covered in the Javadoc, by the way. (Multi has no type-level docs.)

[gavinking]

be easier to discuss if @gavinking himself opened a PR with the API proposal so we can discuss it while trying it.

I don't have a specific proposal because I don't understand the requirements well enough. I can only tell you what is making me uncomfortable in the current design...

... and that I don't think it's a binary choice between "one interface with 350 methods" and "what we have right now". (Again, I could be wrong.)

[cescoffier]

@gavinking Here is a small excerpt from the Uni API. Many groups have been omitted (await, subscribe...), and some subgroups are omitted too (Name with ...).

If you move all these methods a single class, you end up with (I can't remember the exact number) but roughly 350 methods.

[FroMage]

... and that I don't think it's a binary choice between "one interface with 350 methods" and "what we have right now". (Again, I could be wrong.)

Me neither, but your proposal was a bit too abstract for me to follow. An implementation I could try ;) Even if the methods don't do anything, BTW. As long as interfaces are defined, I can try them on for size without running them.

[cescoffier]

So, ok, you don't like async, so what about:

• apply / applyUni / applyMulti / applyCompletionStage / invoke and invokeUni for Uni.onItem()
• apply / applyUni / applyUniAndMerge / applyUniAndConcatenate / applyMulti / applyMultiAndMerge / applyMultiAndConcatenate / applyCompletionStage / invoke and invokeUni for Multi.onItem()

Alternative:

applyUni -> switchToUni (I'm not sure it express the fact you take the item), but it does not work well with AndMerge and AndConcatenate.

[gavinking]

Here is a small excerpt from the Uni API.

Alright, so that's helpful, thanks.

And I think can already begin to discern some of those operations I which I would consider "non-fundamental".

(And I can also see several operations which are duplicated in four places.)

[FroMage]

@cescoffier These are the "basic operations" as I understand them:

1. transform the current item, affecting all subsequent operations on the stream
2. do something with the current item, without affecting the rest of the stream
3. handle a failure, replacing the failure with an item
4. do something with the current item and the current failure, if any, without affecting the rest of the stream

The way I see it, there's mostly the need to replace current imperative language constructs:

1. transform the current item (function composition): your 1. But note that due to sync/async transformations, this can either produce an item or a Uni of an item.
2. invoke a side-effect method without affecting the previous call (the ; operator): your 2.
3. handle a failure by returning an item catch(Throwable){ ... return x; } or per-exception catch(FooException){ ... return x; }
4. handle a failure by returning a different failure catch(Throwable){ ... throw x; } or per-exception catch(FooException){ ... throw x; }
5. handle failure and completion both by invoking a side-effect function finally{ ... }

[cescoffier]

Alright, so that's helpful, thanks.

I should be able to share the editable version with you if you want.

And I think can already begin to discern some of those operations I which I would consider "non-fundamental".

(And I can also see several operations which are duplicated in four places.)

Yes, which would mean the same name, but the listened event is different (so the parameter)

[FroMage]

So, ok, you don't like async, so what about:

apply / applyUni / applyMulti / applyCompletionStage / invoke and invokeUni for Uni.onItem()
apply / applyUni / applyUniAndMerge / applyUniAndConcatenate / applyMulti / applyMultiAndMerge / applyMultiAndConcatenate / applyCompletionStage / invoke and invokeUni for Multi.onItem()

Honestly that would work for me. But again, this is orthogonal with @gavinking 's proposal to groupd operations differently. This is mostly about names, and those proposed names seem fine to me.

[cescoffier]

So, ok, you don't like async, so what about:

apply / applyUni / applyMulti / applyCompletionStage / invoke and invokeUni for Uni.onItem()
apply / applyUni / applyUniAndMerge / applyUniAndConcatenate / applyMulti / applyMultiAndMerge / applyMultiAndConcatenate / applyCompletionStage / invoke and invokeUni for Multi.onItem()

Honestly that would work for me. But again, this is orthogonal with @gavinking 's proposal to groupd operations differently. This is mostly about names, and those proposed names seem fine to me.

This PR was about names, not groups :-)
It drove a discussion about groups, which is interesting.

[gavinking]

The way I see it, there's mostly the need to replace current imperative language constructs:

Right. Precisely.

1. transform the current item (function composition): your 1. But note that due to sync/async transformations, this can either produce an item or a Uni of an item.

See here's where I really feel like I'm viewing these operations from a slightly different perspective.

I feel like you guys are thinking of these operations from the point of view of manipulations on a stream.

But my perspective is, for want of a better way of describing it, the perspective of the Haskell do-notation. That what I'm really modeling here is a sequential computation. That the important thing is not the stream itself, or the item in it, but rather the imperative "steps" that I perform.

Now, of course, these perspectives are obviously equally-valid.

But I think it does explain why I think that apply() and applyUni() are the "same" operation.

[gavinking]

So, ok, you don't like async, so what about:

• apply / applyUni / applyMulti / applyCompletionStage / invoke and invokeUni for Uni.onItem()
• apply / applyUni / applyUniAndMerge / applyUniAndConcatenate / applyMulti / applyMultiAndMerge / applyMultiAndConcatenate / applyCompletionStage / invoke and invokeUni for Multi.onItem()

FTR these names look quite sane to me too.

[gavinking]

(And I can also see several operations which are duplicated in four places.)

Yes, which would mean the same name, but the listened event is different (so the parameter)

Looking more closely at this, I guess the thing that sticks out is how many different operations we have on UniOnTimeout and UniOnFailure. I'm really finding it very difficult to grasp how all these different operations are necessary.

Focusing on exception handling, I feel like I would be better served by a single "handle" method that comes in synch and async flavors.

[cescoffier]

I actually used most of these methods in various app / framework. The only one that can be seen superfluous is onFailure().recoverWithItem(T fallback) because you can use the supplier version. That being said, the supplier would be called on every subscription which is not the case with the item variant.

[cescoffier]

I've renamed the method as listed in #165 (comment)

[jponge]

(catching up after PTO)

I'm the one who came up with the Async variants, because most of the time they are related to some asynchronous operation. Now it's true that this is probably too much and too tied to the execution model, so apply{Type} is fine with me, and things like applyUniAndMerge are meaningful.

[zeljkot]

Why all methods have a parameter type as a suffix? Is the parameter type not enough?

[cescoffier]

Because of Java. It won't see the difference between: ``` Uni<T> method(Function<X, T>); Uni<T> method(Function<X, Uni<T>>); Uni<T> method(Function<X, Multi<T>>); ``` Le jeu. 25 juin 2020 à 22:14, Željko Trogrlić <notifications@github.com> a écrit :

…

Why all methods have a parameter type as a suffix? Is the parameter type not enough? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#165 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADCG7KWYY4BUXISNUU53RTRYOVZ5ANCNFSM4N5BXKOA> .

[cescoffier]

Superseded by the proposal from #178.