RationalJS / future

A Js.Promise alternative for ReasonML
213 stars 15 forks source link

Future.flatMapOk, Future.flatMapError #3

Closed scull7 closed 6 years ago

scull7 commented 6 years ago

I see that FutureResult.flatMapOk and FutureResult.flatMapError were deprecated, however, a replacement methods are not provided. Were these purposely not implemented?

I think the Future as an Either type is most commonly what you want as evidenced by most of the Future/Task implementations I've come across. I like that you're not forced into that pattern, but having the option available is quite a boon in my opinion.

Examples

http://package.elm-lang.org/packages/elm-lang/core/latest/Task https://docs.rs/futures/0.2.1/futures/prelude/trait.Future.html https://github.com/slamdata/purescript-aff https://github.com/folktale/data.task https://github.com/fluture-js/Fluture

gilbert commented 6 years ago

It was intentional, yes. I'm not against adding them, I just wanted to wait until I or someone else has a concrete need for them to base the implementation on. If you have that need, post your use case here for reference and we can get a PR going :)

scull7 commented 6 years ago

Interaction with Js.Promise

This is a pattern that is used quite often with MySql database connections. Also, notice the secondary network call to store the result in a Redis cache.

exception DbError(string);

let dbExec = (db, query, params) =>
  Db.execute(db, query, params)
  |> Js.Promise.then_(res => res |> Belt.Result.Ok)
  |> Js.Promise.catch(error => error |> Js.String.make |> DbError |> Belt.Result.Error)

let thing = (db, input, callback) =>
  dbExec(db, "some query", input)
  |. Util.Future.flatMapOk(res => dbExec(db.mysql, "dependent query", res))
  |. Util.Future.flatMapOk(res => Redis.store(db.redis, res))
  |. Future.mapOk(res => callback(Js.Nullable.null, Js.Nullable.return(res)))
  |. Future.mapError(err => callback(Js.Nullable.return(err), Js.Nullable.null))
  |. ignore;

User provided Future.t(Belt.Result.t('a)) returning functions.

This is code that I'm working with right now.

module Handler = {
  type t = (. Context.t) => (. Call.t, Callback.t) => unit;

  let make = (~metaParser, ~decoder, ~handler, ~encoder) => {
    (
      (. ctx: Context.t) => (. call: Call.t, callback: Callback.t): unit =>
        metaParser(ctx, call)
        |. Util.Future.flatMapOk(meta => decoder(ctx, call, meta))
        |. Util.Future.flatMapOk(req => handler(ctx, call, req))
        |. Util.Future.flatMapOk(res => encoder(ctx, res))
        |. Future.mapOk(json => Callback.success(callback, json))
        |. Future.mapError(exn => Callback.fail(callback, exn))
        |. ignore
    )
  };
};
scull7 commented 6 years ago

Here is my (egregiously copied from @gilbert) implementation for flatMapOk and flatMapError


module Future = {
  open Belt.Result;

  let flatMapOk = (future, f) => Future.flatMap(future, r =>
    switch(r) {
    | Ok(v) => f(v)
    | Error(e) => Future.value(Error(e))
    });

  let flatMapError = (future, f) => Future.flatMap(future, r =>
    switch(r) {
    | Ok(v) => Future.value(Ok(v))
    | Error(e) => f(e)
    });
};
gilbert commented 6 years ago

Very nice, thank you. Do you have time to make a PR with tests? If not then I'll plan to add these in this sometime weekend.

scull7 commented 6 years ago

I'll get to work on that this evening.

scull7 commented 6 years ago

So... I lied. I saw the tests were easy to implement and just went ahead and did it. https://github.com/RationalJS/future/pull/5