Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why do we have Amp\Future #49

Closed
kelunik opened this issue Aug 13, 2016 · 7 comments
Closed

Why do we have Amp\Future #49

kelunik opened this issue Aug 13, 2016 · 7 comments
Labels

Comments

@kelunik
Copy link
Member

kelunik commented Aug 13, 2016

What's the purpose of Amp\Future? It's documented to be used only in internal code, no public APIs?

@kelunik kelunik added this to the v2.0.0 milestone Aug 13, 2016
@kelunik kelunik changed the title Why do we have future Why do we have Amp\Future Aug 13, 2016
@bwoebi
Copy link
Member

bwoebi commented Aug 13, 2016

Amp\Future is meant to be used inside libraries, i.e. it never should cross public API boundaries, correct.

@kelunik
Copy link
Member Author

kelunik commented Aug 13, 2016

Why do we have it now? Why not just Deferred as we had previously?

@bwoebi
Copy link
Member

bwoebi commented Aug 13, 2016

I'm not 100% sure why it's necessary. The only difference is it saving us from a single ->getAwaitable() call. I do not think this is worth it…

But perhaps I'm missing something and @trowski can tell.

@kelunik
Copy link
Member Author

kelunik commented Aug 13, 2016

That's exactly why this issue exists.

@trowski
Copy link
Member

trowski commented Aug 14, 2016

Future (and Producer for observables) only exists to avoid the function call to retrieve the awaitable (or observable) when the object will never be exposed to a public API. It's a minor performance bump, but if the confusion isn't worth it, Deferred and Postponed can be used instead.

@kelunik
Copy link
Member Author

kelunik commented Aug 14, 2016

We didn't need it in Amp v1, I don't think we need it in Amp v2. It adds more confusion than a single method call can save us.

@bwoebi
Copy link
Member

bwoebi commented Aug 18, 2016

Resolved by removal.

@bwoebi bwoebi closed this as completed Aug 18, 2016
kelunik pushed a commit that referenced this issue Mar 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants