add polymorphic parallel type

[cortopy]

Current typings can't determine parallel routine types when array of Futures has polymorphic Rights.

I've followed the pattern of heterogeneous tuples, as per the PromiseConstructor in the Typescript definition for es2015 promises

I'm still wondering though if this could be improved by doing the same for Left (although the combinations may be a very high number) and/or for race.

[codecov-io]

Codecov Report

Merging #184 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master   #184   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          50     50           
  Lines        1092   1092           
=====================================
  Hits         1092   1092

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77da3cf...5031fea. Read the comment docs.

[Avaq]

I think there should be a better solution than the one proposed in the current diff. Could you expand on when array of Futures has polymorphic Rights? What is the input, desired result, encountered type error, etc?

[cortopy]

Basically my problem is that this gives compile error in Typescript because the Rights of the three Futures are of different type

parallel(Infinity, 
      [
        getUser(someUserId), // this is a Future<string, User | null>
        getListOfSomething(someId), // this is a Future<string, string[]>
        findNumberForSomething() // this is a Future<string, number | null>
      ]
    )
    .chain(([user, list, id]) => {})

So I have to explicitly set the polymorphic types in the array and then set the types in the next chain step:

parallel(Infinity, 
      [
        getUser(someUserId),
        getListOfSomething(someId),
        findNumberForSomething()
      ] as Future<string, User | string[] | number | null>[]
    )
    .chain(([user, list, id]: [ User | null, string[], number | null ]) => {})

With the changes submitted, the first case will be OK. And not only that, I don't have to write anything else. Typescript will know the types of user, list, and id

[Avaq]

I'm reluctant to overload parallel for this reason. The goal of the parallel function is to turn an Array (Future a b) into a Future a (Array b). Note that to preserve the consistency of the b type, all items in the array must be of the same type.

At run-time, it is costly (in terms of performance) to enforce this constraint, which is why Fluture doesn't. However, the introduction of TypeScript allows this constraint to be enforced without a cost. Essentially, when you are using TypeScript, you are no longer able to "abuse" the parallel function.

By abuse I mean using it for ends it wasn't intended for. I don't oppose creativity, but I do oppose creativity at the cost of type consistency. ;)

The solution you offer is to overload the parallel function to introduce "tuples" into it. So after your proposed change, the parallel function would operate on ten different types:

1. Array (Future a b)
2. Tuple2 (Future a b) (Future a c)
3. Tuple3 (Future a b) (Future a c) (Future a d)
4. .. 10. Et cetera.

I am of the opinion that API's which use arbitrary overloading like that lead to bugs. The smaller the amount of input types for a function are, the bigger the chance that you can catch a developers mistake through type checking.

---

In conclusion, you are simply using the wrong function to achieve your goal. Let's look at the alternatives, and consider whether they are convenient enough:

The first alternative uses ap. Note that the pattern shown below has been abstracted under the common name lift. If you can find a library offering common algebraic abstractions with TypeScript support, then using lift will make it less verbose.

import {Par, seq, of as resolve} from 'fluture'

//This is how we can combine the results in sequence
resolve((user) => (list) => (id) => {...})
.ap(getUser(someUserId))
.ap(getListOfSomething(someId))
.ap(findNumberForSomething())

//If we want parallelism, like in your original example, we need Par and lift
const combineResults = lift3((user) => (list) => (id) => {...}))
seq(combineResults(
  Par(getUser(someUserId)),
  Par(getListOfSomething(someId)),
  Par(findNumberForSomething())
))

Note that though this way of working gives us a huge amount of control, it's also pretty verbose (and your lift would need TS support), which leads me to the next alternative. both: a function that was designed to return tuples, offers a method closer to your original code, but type safe without overloading:

getUser(someUserId)
.both(getListOfSomething(someId))
.both(findNumberForSomething())
.chain(([[user, list], id]) => {...})

Now with this second example, the only pain point might be that we have to consume nested binary tuples. This brings me back to your original solution. You may have noticed that the signature for your parallel<L, R1, R2> function looks a lot like the signature for both. Since I'm opposed overloading parallel, what if we implement your solution as separate functions, so for example:

function both<L, R1, R2>(Future<L, R1>, Future<L, R2>): Future<L, [R1, R2]>
function all3<L, R1, R2, R3>(Future<L, R1>, Future<L, R2>, Future<L, R3>): Future<L, [R1, R2, R3]>    
function all4<L, R1, R2, R3, R4>(Future<L, R1>, Future<L, R2>, Future<L, R3>, Future<L, R4>): Future<L, [R1, R2, R3, R4]>
function all5<L, R1, R2, R3, R4, R5>(Future<L, R1>, Future<L, R2>, Future<L, R3>, Future<L, R4>, Future<L, R5>): Future<L, [R1, R2, R3, R4, R5]>

This is something I would be open to merge, but it might be a lot of work, especially when you consider currying. And looking at all that work, you might say that consuming nested binary tuples isn't so bad. ;)

[cortopy]

That's a very interesting read, and a great insight into what you had in mind for Fluture.

A few comments on what you say:

At run-time, it is costly (in terms of performance) to enforce this constraint, which is why Fluture doesn't. However, the introduction of TypeScript allows this constraint to be enforced without a cost. Essentially, when you are using TypeScript, you are no longer able to "abuse" the parallel function.

I'm not sure that's 100% true. Arrays in a polymorphic type can be cast into Tuples. There's nothing in Typescript that wouldn't allow this type casting, and so parallel can still be used with a tuple of Futures if you cast the array as per my example. It's verbose, but it's allowed.

The solution you offer is to overload the parallel function to introduce "tuples" into it. So after your proposed change, the parallel function would operate on ten different types

Just like Promise.all. When I started using Flutture, I thought parallel was an equivalent for it. Reading your comment it sounds like Flutture was designed not to have an equivalent to Promise.all?

I am of the opinion that API's which use arbitrary overloading like that lead to bugs. The smaller the amount of input types for a function are, the bigger the chance that you can catch a developers mistake through type checking.

Totally agree. Overloading feels very bad, but I wonder if that's one limitation of Typescript we have to deal with. For what I have seen, it's quite a common practice. Not only does Promise.all use it, Ramda types also make use of the same technique. See for example this interface from Ramda:

interface CurriedFunction6<T1, T2, T3, T4, T5, T6, R> {
        (t1: T1): CurriedFunction5<T2, T3, T4, T5, T6, R>;
        (t1: T1, t2: T2): CurriedFunction4<T3, T4, T5, T6, R>;
        (t1: T1, t2: T2, t3: T3): CurriedFunction3<T4, T5, T6, R>;
        (t1: T1, t2: T2, t3: T3, t4: T4): CurriedFunction2<T5, T6, R>;
        (t1: T1, t2: T2, t3: T3, t4: T4, t5: T5): (t6: T6) => R;
        (t1: T1, t2: T2, t3: T3, t4: T4, t5: T5, t6: T6): R;
    }

Reading your two solutions, I don't understand why both can be a Pair/Tuple and parallel can't. I suppose you're following a pattern I'm not aware of, but having different methods, types and expected behaviour depending on length of array feels odd to me.

The other solution feels far too verbose for me. I think I'm going to follow the premise that simplicity is better in every use case. I feel like I'm going to stick to parallel even if you didn't intend it to be used this way (sorry for that!).

[Avaq]

That's a very interesting read, and a great insight into what you had in mind for Fluture

Thank you for taking the time to understand my points of view. :)

parallel can still be used with a tuple of Futures if you cast the array as per my example.

Right, but at least TypeScript has made you aware that you are working with inconsistent types, and it gave you the chance to correct this potential mistake.

Flutture was designed not to have an equivalent to Promise.all?

Fluture aims to provide a safer alternative, rather than a straight equivalent. Having to explicitly pass Infinity for (risky) limitless concurrency and being discouraged to use mixed type arrays are both part of the "safer" design.

For what I have seen, [overloading is] quite a common practice. [...] See for example this [CurriedFunction6] interface from Ramda.

I don't oppose using TypeScripts overloading feature to describe API using things such as currying. I wish there were a less verbose alternative, but as of yet overloading is the best we have. I do oppose the common practice of arbitrarily* overloading a function that does one thing, with a different behaviour.

* By arbitrary overloading, I mean cases where functions are overloaded just because the behaviour feels similar, or the verb you used for your function happens to also relate to the second behaviour, or the function has a convenient short name, etc. Examples of this are Promise#then (which is like map, chain, fork, mapRej, and chainRej all overloaded into a single function) but also older API's like jQuery() (which when given a function binds it as a listener to window.onload, and when given a string queries the document for elements, etc). In the latter example, the API was overloaded because of (I assume) the convenient short function name ($(...)). In my opinion this is bad API design as it favours convenience over type consistency, and introduces complexity as a result, defeating the original motivation of simplicity. This happens because people confuse ease with simplicity. I'll come back to that.

Arrays in a polymorphic type can be cast into Tuples. [...] Reading your two solutions, I don't understand why both can be a Pair/Tuple and parallel can't.

To me, the fact that I can cast one data type to another doesn't mean that my functions should by definition be able to operate on both. I still prefer distinguishing between functions that operate on Tuples, and functions that operate on Arrays (despite being able to cast between them, and in fact that they are the same in JavaScript). Let's look at the type signatures for the two functions (ignoring the concurrency limit for brevity):

parallel :: Array (Future a b) -> Future a (Array b)
both     :: (Future a b, Future a c) -> Future a (Tuple b c)

They are very similar in use:

parallel ([ Future.of(1), Future.of(2) ]).map(([a, b]) => a + b)
both     (  Future.of(1), Future.of(2)  ).map(([a, b]) => a + b)

...but their types are very different: parallel operates on Arrays, whereas both operates on Tuples, parallel is intended for Arrays of arbitrary length (hence the concurrency limit) and both is intended for cases where you know the limit. The only problem is that both has only been implemented for tuples of size two, so users are forced to nest their tuples if they need more space (as shown in my second alternative). This problem led me to suggest implementing the same function for bigger tuples, like:

both :: (Future a b, Future a c) -> Future a (Tuple2 b c)
all3 :: (Future a b, Future a c, Future a d) -> Future a (Tuple3 b c d)
all4 :: (Future a b, Future a c, Future a d, Future a e) -> Future a (Tuple4 b c d e)

I think I'm going to follow the premise that simplicity is better in every use case.

I feel it's important to distinguish simplicity from ease of use. I feel like overloading the parallel function makes for a complex API that is (at the surface) easy to use. The both API is simple, but not quite as easy to use because of the nested tuples. The suggested all* API introduces an easy-to-use solution which does not compromise simplicity. The question I implicitly posed at the end of my previous response asked whether you think it's worth extending the API to make it easier, when you contrast the extra work going into the creation of this API against the extra work of destructuring nested tuples.

In my opinion, it's not worth it, which is why I haven't implemented these functions. However, your opinion might differ on the matter and I welcome a pull-request that introduces them.

[cortopy]

I agree with you that writing code just to avoid the overload is a lot of work for a little gain in convenience.

I think it might also be a bad idea because the API would be developed in response to Typescript idiosyncrasies and not all users use Typescript. The issue this pull request tried to resolve is not present in vanilla JS anyway.

I'm personally going to stick to array/tuple conversion. I know Fluture wasn't intended this way, but the compromise is worth it for me