Giter VIP home page Giter VIP logo

Comments (41)

kennetpostigo avatar kennetpostigo commented on May 25, 2024 9

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.

from promise.

johnhaley81 avatar johnhaley81 commented on May 25, 2024 9

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

šŸšŖšŸƒā€ā™‚ļø

from promise.

rizo avatar rizo commented on May 25, 2024 8

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.

from promise.

rizo avatar rizo commented on May 25, 2024 8

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?

from promise.

rizo avatar rizo commented on May 25, 2024 7

@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.

from promise.

thangngoc89 avatar thangngoc89 commented on May 25, 2024 6

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

from promise.

johnhaley81 avatar johnhaley81 commented on May 25, 2024 4

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.

from promise.

aantron avatar aantron commented on May 25, 2024 4

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 :)

from promise.

rizo avatar rizo commented on May 25, 2024 3

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

from promise.

aantron avatar aantron commented on May 25, 2024 3

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

from promise.

arnarthor avatar arnarthor commented on May 25, 2024 3

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.

from promise.

aantron avatar aantron commented on May 25, 2024 3

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.

from promise.

aantron avatar aantron commented on May 25, 2024 2

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

from promise.

danny-andrews-snap avatar danny-andrews-snap commented on May 25, 2024 2

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. :)

from promise.

ncthbrt avatar ncthbrt commented on May 25, 2024 1

|. Fast pipe

Pros

  • zero cost

Cons

  • not available using Ocaml syntax

|> Pipe

Pros

  • works everywhere

Cons

  • indirection
  • worse type inference due to indirection

from promise.

arnarthor avatar arnarthor commented on May 25, 2024 1

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.

from promise.

thangngoc89 avatar thangngoc89 commented on May 25, 2024 1

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

from promise.

rizo avatar rizo commented on May 25, 2024 1

Elm uses and_then which is ok too.

from promise.

rizo avatar rizo commented on May 25, 2024 1

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)

from promise.

aantron avatar aantron commented on May 25, 2024 1

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

from promise.

lpil avatar lpil commented on May 25, 2024 1

+1 to flatMap to be consistent with Belt.

from promise.

ivg avatar ivg commented on May 25, 2024 1

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.

from promise.

lpil avatar lpil commented on May 25, 2024 1

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

from promise.

aantron avatar aantron commented on May 25, 2024 1

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

from promise.

ncthbrt avatar ncthbrt commented on May 25, 2024

|. 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.

from promise.

aantron avatar aantron commented on May 25, 2024

@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 |..

from promise.

ncthbrt avatar ncthbrt commented on May 25, 2024

|. 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)

from promise.

aantron avatar aantron commented on May 25, 2024

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

from promise.

aantron avatar aantron commented on May 25, 2024

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.

from promise.

aantron avatar aantron commented on May 25, 2024

@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.

from promise.

aantron avatar aantron commented on May 25, 2024

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).

from promise.

aantron avatar aantron commented on May 25, 2024

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 |.?

from promise.

aantron avatar aantron commented on May 25, 2024

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

from promise.

rizo avatar rizo commented on May 25, 2024

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.

from promise.

aantron avatar aantron commented on May 25, 2024

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);

from promise.

arnarthor avatar arnarthor commented on May 25, 2024

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

from promise.

aantron avatar aantron commented on May 25, 2024
  • 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.

from promise.

lpil avatar lpil commented on May 25, 2024

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.

from promise.

aantron avatar aantron commented on May 25, 2024

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

from promise.

lpil avatar lpil commented on May 25, 2024

So map should be then? :)

from promise.

aantron avatar aantron commented on May 25, 2024

butThen :)

from promise.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    šŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. šŸ“ŠšŸ“ˆšŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ā¤ļø Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.