Coder Social home page Coder Social logo

Comments (20)

joshdifabio avatar joshdifabio commented on August 24, 2024 2

In some contexts onComplete makes more sense and in some contexts onResolve makes more sense, but promises resolve, they don't complete. However, I think part of the issue here is that you are implementing Promise directly in places where you shouldn't. For example, I don't think it makes sense for a stream to implement Promise directly as a stream isn't primarily a placeholder for some single future value.

function ($stream) {
    yield $stream; // this doesn't read well at all. What are we even waiting for here?
}

I would prefer this:

function ($stream) {
    yield $stream->awaitEnd(); // or whenEnded()
}

Regarding the name when(), I'm struggling to think of a context in which it makes sense:

a()->when($b); // in English this reads as "do $a when $b", which is the opposite of what the code does

from amp.

kelunik avatar kelunik commented on August 24, 2024

Specifically when just tells that it's something event based, but not on which event the callback is fired. Could be when new data is available on a stream or when there's an error or when …. onResolve clearly indicates which event happens.

from amp.

J7mbo avatar J7mbo commented on August 24, 2024

onComplete definitively means finished. Other Apis that make requests typically use onComplete. onResolve is using yet more terminology that makes entry into things like amp even more prohibitive. Complete > Resolve.

from amp.

kelunik avatar kelunik commented on August 24, 2024

@J7mbo Somebody mentioned complete would indicate success. I don't necessarily think so, just mentioning in case others feel that way.

from amp.

PeeHaa avatar PeeHaa commented on August 24, 2024

complete sounds like the complete action itself instead of the event being fired.

from amp.

J7mbo avatar J7mbo commented on August 24, 2024

@kelunik Well take a look at jQuery (great example, I know!), I think we had onSuccess, onFail and onComplete, the latter was always executed. Then again that's now .success, .error and .always so not entirely a great comparison. Complete is a positive word so maybe it indicates success. How about onFinish?

from amp.

kelunik avatar kelunik commented on August 24, 2024

@PeeHaa A promise is a placeholder for the outcome of an operation. And that operation is complete then. It won't change anymore.

@J7mbo They still have complete. I like onComplete more than onFinish, can't really tell why. Neither does really fit with Promise though. And we already have resolve on the Deferred part.

from amp.

bwoebi avatar bwoebi commented on August 24, 2024

Also ping @rdlowrey

from amp.

kelunik avatar kelunik commented on August 24, 2024

@joshdifabio One such example of (optional) emits as a stream is Artax. The main purpose of request is to return the response, but it emits updates as Artax opens sockets, completes TLS handshakes and follows redirects.

from amp.

PeeHaa avatar PeeHaa commented on August 24, 2024

@kelunik

And that operation is complete then.

My point was that naming the method complete tells me it's the action if completing something instead of onComplete telling me it's an event based on the completion.

from amp.

kelunik avatar kelunik commented on August 24, 2024

Oh, I didn't mean complete as a method name, but the complete in onComplete.

As we have Deferred::resolve, Promise::onResolve makes sense.

from amp.

kelunik avatar kelunik commented on August 24, 2024

@bwoebi You wanted 2-3 days to think about it. How do you feel now?

from amp.

bwoebi avatar bwoebi commented on August 24, 2024

I still really don't like it.
I do not feel like it helps understanding anything.
For random API parts, more expressive names are typically fine.
when() however is quite a unique part of the API. It's not your typical callback, but you need to exactly know what it is. Calling it onResolve just tells you a small part of its function. But it lets you ask another question: "When is a Promise resolved? What will resolve it?" As such, you either way need to have a closer look at the documentation, at which point there's no advantage of calling it onResolve() vs when().
when() is simply just short and concise. Moreover when() sounds a bit like then(), suggesting for people from other Promise APIs that it must be related, making it easier for them.
onResolve() feels like a step backwards for me.

from amp.

PeeHaa avatar PeeHaa commented on August 24, 2024

Not saying I disagree with the gist @bwoebi, but saying something sounds like something else to make something more clear is weird imo.

from amp.

bwoebi avatar bwoebi commented on August 24, 2024

It's just a single advantage, not the whole argument ;-)
It's more the type of thing which would make me decide on something if all other advantages and disadvantages are about equivalent.

from amp.

kelunik avatar kelunik commented on August 24, 2024

I do not feel like it helps understanding anything.

It makes the API consistent. We have Deferred::resolve, why introduce a new term when instead of using the existing API name?

when() however is quite a unique part of the API. It's not your typical callback, but you need to exactly know what it is.

You can bring that argument for every single name. It's not an argument.

when() is simply just short and concise. Moreover when() sounds a bit like then(), suggesting for people from other Promise APIs that it must be related, making it easier for them.

Indeed, and that's an argument against using when(). Because it's really not then(), but rather similar to done(). It's not chainable, you shouldn't throw in it, and it doesn't make sense to return a value from it.

onResolve() feels like a step backwards for me.

Could you explain why it's backwards?

from amp.

bwoebi avatar bwoebi commented on August 24, 2024

It makes the API consistent. We have Deferred::resolve, why introduce a new term when instead of using the existing API name?

The Deferred is just one implementation, which returns a Promise when some method is called. It's a separate API from Promise. It makes sense to model the naming of the Deferred methods after the method(s) of the Promise, but not vice-versa.

You can bring that argument for every single name. It's not an argument.

Most APIs do not have so much complexity behind. - No.

Indeed, and that's an argument against using when(). Because it's really not then(), but rather similar to done(). It's not chainable, you shouldn't throw in it, and it doesn't make sense to return a value from it.

I just disagree. If it were the same, we'd call it then(). Throw/return/chain are just one thing: it transforms the promise. There's the only difference. We do not return a transformed promise. For everything else, it's quite the same thing.

Could you explain why it's backwards?

Our current naming is totally fine and you're proposing something worse…

from amp.

kelunik avatar kelunik commented on August 24, 2024

The Deferred is just one implementation, which returns a Promise when some method is called. It's a separate API from Promise. It makes sense to model the naming of the Deferred methods after the method(s) of the Promise, but not vice-versa.

It's the standard implementation. resolve + onResolve work fine together, it's clear from the name that they're related. Can you propose something for Deferred matching when?

Throw/return/chain are just one thing: it transforms the promise. There's the only difference.

It's not the only difference. The second difference is one callback instead of two. And transforming the promise or not is exactly the difference between then and done.

Most APIs do not have so much complexity behind. - No.

Our promises aren't really complex, they're quite simple, as they don't have any chaining or something like that. They just fire a simple callback (or a set of them) on resolution.

from amp.

bwoebi avatar bwoebi commented on August 24, 2024

Can you propose something for Deferred matching when?

resolve perfectly matches whenwhen callbacks are triggered basically upon the state change of the Promise. That state change is triggered by resolving, so that's totally fine.

It's not the only difference. The second difference is one callback instead of two. And transforming the promise or not is exactly the difference between then and done.

Well, I never liked the name done() … But I'd even prefer done() over onResolve() … it's just too clumsy for me...

Our promises aren't really complex, they're quite simple, as they don't have any chaining or something like that. They just fire a simple callback (or a set of them) on resolution.

Tell that someone who's never interacted with Promises in his life ;-)

from amp.

kelunik avatar kelunik commented on August 24, 2024

the state change of the Promise

In that case our streams shouldn't extend Promise.

Well, I never liked the name done() … But I'd even prefer done() over onResolve() … it's just too clumsy for me...

Using done is only possible if we use two callbacks, otherwise it's not compatible with other existing implementations.

Tell that someone who's never interacted with Promises in his life ;-)

Not being able to understand things is mostly not due to promises, but rather due to generators or not understanding the event loop is the scheduler.

from amp.

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.