Bikeshedding is welcome!
aantron opened this issue ยท 41 comments
Shouldnew_
be renamedmake
to avoid the underscore?- Should Repromise support
|>
or|.
for chaining? - Should we rename
then_
? ToflatMap
?andThen
? resolve
orresolved
?- Should
all
andrace
take lists or arrays? - etc.
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.
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.
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
.
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
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.
@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 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.
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);
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
+1 to flatMap
to be consistent with Belt.
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.
Given a qualified name I will think that flatMap
is the clearest and most consistent name.
Repromise.resolve
is nowRepromise.resolved
(c7c622f).Repromise.Rejectable.reject
also becameRepromise.Rejectable.rejected
.- I merged a PR to rename
then_
toandThen
(#27, thanks @jihchi!). We might have to do a poll or something to figure out whether to go withandThen
orflatMap
. However, sinceandThen
is a unique identifier, it is straightforward to do a blind search-replace to change it toflatMap
later.
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.
So map
should be then
? :)
butThen
:)
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_
tomake
then_
toandThen
resolve
toresolved
reject
torejected
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 :)