Comments (20)
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.
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.
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.
@J7mbo Somebody mentioned complete
would indicate success. I don't necessarily think so, just mentioning in case others feel that way.
from amp.
complete
sounds like the complete action itself instead of the event being fired.
from amp.
@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.
@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.
Also ping @rdlowrey
from amp.
@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.
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.
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.
@bwoebi You wanted 2-3 days to think about it. How do you feel now?
from amp.
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.
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.
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.
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.
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.
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.
Can you propose something for Deferred matching when?
resolve
perfectly matches when
… when
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.
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)
- Return self in Future::ignore HOT 1
- UnhandledFutureError is hard to debug HOT 2
- Migration guide for combinator functions HOT 5
- FiberError: Cannot switch fibers in current execution context on Laravel HOT 4
- 32 bit PHP: Return value of Amp\Loop\NativeDriver::now() must be of the type int, float returned HOT 5
- Functions should require callable instead of closures HOT 10
- Any way to create or get a context for coroutine. HOT 5
- Version confusion HOT 8
- Shouldn't Future::finally() pass the future data to the provided callback? HOT 7
- Event loop terminated without resuming the current suspension (the cause is either a fiber deadlock, or an incorrectly unreferenced/canceled watcher) HOT 3
- Future combinators HOT 8
- When to not to use `async`?
- Comparison with ReactPHP HOT 8
- The docs not up to date? HOT 3
- Where is this `any()` function?
- Interception of pause / resume future HOT 2
- Add Payload::readEntireBody? HOT 3
- Symfony Messenger Integration HOT 4
- Using AMPHP with Psalm HOT 5
- onComplete event for async? HOT 9
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
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.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from amp.