aantron/promise

Bikeshedding is welcome!

aantron opened this issue ยท 41 comments

Please do discuss any such issues. We want to make a pleasant API. We are keeping the Repromise version in 0.x.y for now to allow plenty of room for breakage :)

|. is the recommended form of chaining currently, so would advocate for fast pipe support. This has the downside of not being that pleasant in bucklescript/OCaml, but results in less indirection and better type inference on the reason side of things.

@ncthbrt Is |. BuckleScript/Reason-only, though? If so, we would have to stick with |> for now, to keep Repromise native support. We'd probably have to upstream |. into OCaml, and then we can switch over the Repromise API to use |..

|. is Reason only.

Is |> a rewrite in native, or is it implemented as a function call?
Does a |> b become b(a) or is |> implemented as let (|>) = a f => f(a)

|> is a function call, implemented in Pervasives: http://caml.inria.fr/pub/docs/manual-ocaml/libref/Pervasives.html#VAL(|>).

So basically, |. is available on native, but only if using Reason. So it's not a matter of native vs. JS, but OCaml vs. Reason syntax.

|. Fast pipe

Pros

  • zero cost

Cons

  • not available using Ocaml syntax

|> Pipe

Pros

  • works everywhere

Cons

  • indirection
  • worse type inference due to indirection

I would hope _new gets renamed to make since make is used nearly everywhere else. Also I think |> should be what is used, at least for now. Maybe |. support can come later, I don't think it's urgent.

I agree with make instead of new_ since it's pretty much standard for most libraries and bindings in Reason.

I'm torn with the piping, |. is highly encouraged in bucklescript because of no overhead but if it doesn't work in OCaml it's a decision of having nice syntactical support in OCaml or better runtime support in Reason + Bucklescript.

I'm not sure how important the runtime support is, but Hongbo has been pretty actively telling people to use fast pipe. For example all of Belt uses the fast pipe API design and even that is written in OCaml so the decision is pretty obvious in that case which is more important according to the maintainers.

rizo commented

make all the things

I think make should become the new convention for constructor functions of a given module in Reason/OCaml. Examples: Person.make, Service.make, Repromise.make, etc.

Some libraries in OCaml use create, init or even v. make is better IMO.

There's only one way to pipe

Regarding the pipe operator. The less operators we have the better. I strongly prefer |> because it's a simple language-level construct. It's indirection cost is negligible. The type inference should be improved in the compiler. Introducing a new operator is a hack because this problem still exists for other similar operators.

By not forcing the main type to be the first argument, |> works better with partial application.

rizo commented

Oh, and isn't ' better for names that collide with keywords? I think I prefer then' to then_.

@rizo People coming from JS might be reluctant to use ', also it's more difficult to see (which could be an advantage or disadvantage). I prefer synonyms to avoid keywords. @thangngoc89 on Discord has suggested later, next. We could even resort to flatMap.

@aantron @rizo and ' pretty much brokes the syntax highlighting

Ok, since pretty much everyone here and in Discord wants new_ to become make, I opened an issue about it (#23). Leaving it available, in case someone wants to use it to get familiar with the code. Otherwise, I'll do it when Repromise is about to have some other breakage (which will be 0.6.0).

@aantron I think flatMap is a better ideas than my useless bikeshedding about later and next. flatMap describes exactly what it does

rizo commented

Elm uses and_then which is ok too.

I'd personally prefer flatMap & map with infix operators as >>= and <$> respectively.

๐Ÿšช๐Ÿƒโ€โ™‚๏ธ

A little trigger happy on the submit button

Although the |. operator seems to be preferred infix for piping right now with things like Belt, it feels really magical to me while |> is "just a function" so to me it seems easier to reason about and understand what's happening. I also prefer data last.

However, I also heavily subscribe to the "McDonald's Philosophy" of coding which states:

It's better to be consistently bad than inconsistently good.

So even if I think |> is superior, it seems like |. has the traction and I'd rather be consistent.

I think the best thing for moving on |. is to try to get it into the regular OCaml, since it requires special support (as @rizo and @johnhaley81 pointed out). @bobzhang, do you know if there has already been an effort to upstream |.?

...aaand new_ is now make, courtesy of @johnhaley81 in #24 (thanks!). Will release this in a couple days.

rizo commented

@aantron I have to say that Iโ€™m against adding |. to OCaml. Specially because we already have |> which works extremely well with partial application. What I suggest is starting a discussion to address the inference issues associated with |> instead of adding new syntax.

I'm thinking to rename then_ to andThen, Elm-style, as suggested by @rizo.

I think flatMap would be better, not because it's more descriptive, but because it's consistent with other libraries in the eco system (for example Belt).

Adding yet another term can be confusing, especially for newcomers, but also for those who are used to using flatMap in other libs.

The issue I have with flatMap is that it if I am new to promises, it will make no sense to me how the name flatMap describes the operation that is done, nor what is the analogy between promises and options/results that justifies re-using the name. I realize you agree it's not descriptive, but I suggest the analogy doesn't hold that obviously, either. The types are similar, so it obviously holds on a type level, but I think the analogy is quite a bit weaker when thinking of semantics.

flatMap in option/result "looks," to a newcomer, relative to map, like just a nifty convenience function for collapsing ("flattening") options, while flatMap in promises is the fundamental function that allows scheduling more async computation in the future, whereas map schedules synchronous computation. That's why I find andThen appealing. It somehow captures the "additional async" semantics of the function.

Another concern I have is that when mixing options/results and promises, it might actually be beneficial to have two separate (and highly descriptive) names for chaining them, so that it's immediately clear which identifier refers to which datatype being manipulated. This might also be useful when modules are locally opened.

rizo commented

The underlying abstract meaning of the operation is bind from the Monad interface. Scala uses flatMap but bind is more common in OCaml and Haskell. Even if repromise implements the Monad interface (with return, bind), I think having a more friendly alias like andThen is very useful.

I think I'll indeed change it to andThen for now, at least for the next release :)

Also, resolve is named that way by analogy with JS Promise.resolve, but I think resolved is a better name.

resolve seems like it was an attempt in JS to conflate returning a resolved promise with the action of resolving the final promise, in the usage return Promise.resolve(foo);

rizo commented

I think resolved makes more sense indeed. What about Promise.of(42)?

Other options are:

  • Promise.const(42)
  • Promise.return(42)
  • Promise.pure(42)
  • Promise.with_value(42)

I like Promise.return(42) but resolved is good as well. Both are really descriptive of what is happening. I think the others mentioned @rizo are slightly unorthodox

@rizo of is a keyword, so it won't work AFAIK.

lpil commented

+1 to flatMap to be consistent with Belt.

ivg commented

We should consider names qualified, i.e., in the way how they will look in the code. So here are alternatives (I personally hate camelCase and in general primitives that need more than one word in their name):

Promising (nothing new_)

  • Repromise.make ()
  • Repromise.init ()
  • Repromise.create ()
  • Repromise.fresh ()

Chaining (except then_)

  • promise |> Repromise.after (action)
  • promise |> Repromise.next (action)
  • Repromise.upon (promise, action) (for the flipped order of arguments)
  • promise |> Repromise.therefore (action)
  • promise |> Repromise.that (action)
  • promise |> Repromise.chain (action)

Fulfilling (it is resolved)

  • Repromise.fulfilled (value)
  • Repromise.resolved (value)
  • Repromise.finished (value)
  • Repromise.return (value)
  • Repromise.pure (value) (for mathematicians)
  • Repromise.now (value)

Not resolve definitely, as resolve is the name for a function that is the second element of a tuple returned by new_ (besides, did you consider making it abstract? It may save you a lot of hassle in the future)

Note, the order of suggestions is random.

lpil commented

Given a qualified name I will think that flatMap is the clearest and most consistent name.

  • Repromise.resolve is now Repromise.resolved (c7c622f). Repromise.Rejectable.reject also became Repromise.Rejectable.rejected.
  • I merged a PR to rename then_ to andThen (#27, thanks @jihchi!). We might have to do a poll or something to figure out whether to go with andThen or flatMap. However, since andThen is a unique identifier, it is straightforward to do a blind search-replace to change it to flatMap later.
lpil commented

If we want a more specific time based name than flatMap do we want to do the same for map? It also applies the given function to the contained value after the promise has resolved, and also has a math-y name that describes the behaviour in relation of a rather abstract Functor type.

Thinking about the relationship between map and flatMap I don't see why flatMap is more suited to the name andThen, it says nothing about chaining on another async computation, which is what makes flatMap/andThen special.

@lpil for me it's the and, it is more async after async.

lpil commented

So map should be then? :)

butThen :)

rizo commented

Oh my... ๐Ÿ˜„ If I had to vote I'd just go for map and bind โ€“ the traditional names well understood in the community. I can see how andThen is more convenient, but please don't change map.

Aren't people supposed to use the ppx syntax anyway for binds?

Ok, I released Repromise 0.6.0, with these renamings for now:

  • new_ to make
  • then_ to andThen
  • resolve to resolved
  • reject to rejected

Thanks for the discussion so far :)

The underlying abstract meaning of the operation is bind from the Monad interface. Scala uses flatMap but bind is more common in OCaml and Haskell. Even if repromise implements the Monad interface (with return, bind), I think having a more friendly alias like andThen is very useful.

A few thoughts on this. First, aliases should be considered as a last resort because, although they are cheap from an implementor's POV, they are very expensive from a consumer's. At best, they require a special linting rule (if one even exists) to ensure your team uses one name over the other consistently, and at worst they cause unnecessary cognitive load when reading code that uses them interchangably. (I can't count the number of times I had to consult the docs to reassure myself that inject and reduce were equivalent in Ruby.) It's a HUGE PITA! (A concise argument against aliases can be found here.)

I think we ought to choose a name that is part of the monad interface, and avoid aliases altogether. I'm not sure if there is any such spec in Reason, but there exists an interface specification in JS that we could leverage: https://github.com/fantasyland/fantasy-land#monad. This spec suggests the use of chain in place of flatMap or andThen. I think this is a great choice since it is general enough to be applied to any type of monad (not just promises) and it is intuitive (you use it to chain operations).

Let me know what you guys think. :)

Closing this as it seems settled for the time being, however bikeshedding is still welcome :)