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

Missing helper (or am I missing something?) #39

Open
pckilgore opened this issue Nov 24, 2019 · 2 comments
Open

Missing helper (or am I missing something?) #39

pckilgore opened this issue Nov 24, 2019 · 2 comments

Comments

@pckilgore
Copy link

pckilgore commented Nov 24, 2019

First thank you for this library, it's been a joy to work with over Js.Promise.

A question: I feel like I'm constantly reaching for functions of signature:

let tryIfOk: (Future.t(Belt.Result.t('c, 'b)), 'c => Belt.Result.t('a, 'b)) => 
  Future.t(Belt.Result.t('a, 'b))

let tryIfErr: (Future.t(Belt.Result.t('b, 'c)), 'c => Belt.Result.t('a, 'b)) => 
  Future.t(Belt.Result.t('a, 'b))

Is there a simple way to compose this out of other functions already provided? It seems like no matter how I go about this in my code I'm doing a lot of ugly manual wrapping (which does work, it just feels extremely inelegant).

Thank you!

@scull7
Copy link
Collaborator

scull7 commented Nov 24, 2019

You can achieve a similar result if you "lift" your function into a future.

let lift: ('c => Belt.Result.t('a, 'b), 'c) => Future.t(Belt.Result.t('a, 'b));

let lift = (fn, c) => Future.value(fn(c));

let futureResult = someFuture -> Future.flatMapOk(lift(myFn))

But yes, I think I agree that this is missing a function. Possibly named andThen as I've done here:
https://scull7.github.io/bs-result/api/Result_promise.html#value-andThen

@pckilgore
Copy link
Author

Interesting, thanks. Funny enough I'm using something a bit different, never really thought hard if I had made a poor trade-off (other than limited re-usability over composing with lift).

let andIfOk = (fut, f) =>
  Future.map(fut, res => Belt.Result.flatMap(res, f));

Happy to chip in a PR for discussion if there is some consensus around adding a new function.

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

No branches or pull requests

2 participants