Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add map2...map5 for Future and Future of Result #23

Merged
merged 1 commit into from
Nov 13, 2018

Conversation

mlms13
Copy link
Contributor

@mlms13 mlms13 commented Oct 21, 2018

These functions allow you to wait for multiple futures to resolve and "map" the separate values from all of those futures into a new value.

Here is the corresponding functionality for the Task type in Elm as well as a similar idea in this promise library for Haxe.

I chose to use the mapN name (as Elm does) because it seems more easily-understandable than a name like liftA2 used in Haskell. I also chose to implement map5 in terms of map4 mostly because it's much more terse than lots of nested flatMaps, but the code might be a little less readable as a result.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 29

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.4%) to 98.387%

Totals Coverage Status
Change from base Build 26: 0.4%
Covered Lines: 109
Relevant Lines: 110

💛 - Coveralls

2 similar comments
@coveralls
Copy link

Pull Request Test Coverage Report for Build 29

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.4%) to 98.387%

Totals Coverage Status
Change from base Build 26: 0.4%
Covered Lines: 109
Relevant Lines: 110

💛 - Coveralls

@coveralls
Copy link

Pull Request Test Coverage Report for Build 29

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.4%) to 98.387%

Totals Coverage Status
Change from base Build 26: 0.4%
Covered Lines: 109
Relevant Lines: 110

💛 - Coveralls

@seprich
Copy link
Contributor

seprich commented Oct 22, 2018

Have you compared your work with the Js.Promise equivalents all2,.. all6 in https://bucklescript.github.io/bucklescript/api/Js.Promise.html ?
The nice thing about the standard bucklescript implementation is the fact that it can be chained with the arrow operator because tuples are used. Also it kind of sets precedent for the preferable naming in my mind but that can be debated. But the main difference is the signature.
Otherwise, especially the mapOk2, etc (or allOk2, ...) functions are IMO very important for effective use of this lib.

@mlms13
Copy link
Contributor Author

mlms13 commented Oct 22, 2018

@seprich it would be pretty easy (and probably very worthwhile) to add all2...all6 written in terms of map2...map6, but i think the mapN functions are valuable enough on their own (and a pretty common fixture in functional libraries in general... there's been talk of adding similar mapN functions to Belt.Option and Belt.Result).

Here's a case where it's very useful to have function-of-arity-n instead of tuple-n:

type userWithOrders = {
  user: User.t,
  orders: list(Order.t)
};

let make = (user, orders) => { user, orders };

let futureOfUserWithOrders =
  Future.mapOk2(getUserById("abc"), getOrdersForUserId("abc"), make);

If we want, I'm happy to add several allN functions, which would basically be defined as:

let all3 = (fa, fb, fc) => map3(fa, fb, fc, (a, b, c) => (a, b, c));

@gilbert
Copy link
Member

gilbert commented Nov 13, 2018

This looks great. It's a useful utility that one shouldn't have to write on their own. Thanks!

@gilbert gilbert merged commit e16ed7a into RationalJS:master Nov 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants