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

[RFC] Functional API #5

Closed
clue opened this issue Jul 2, 2015 · 4 comments
Closed

[RFC] Functional API #5

clue opened this issue Jul 2, 2015 · 4 comments

Comments

@clue
Copy link
Owner

clue commented Jul 2, 2015

The current OOP API looks a bit flawed IMO.

This ticket serves as a discussion basis whether it makes sense to use plain functions instead.

Current API:

$promise = React\Promise\resolve(123);
$blocker = new Clue\React\Block\Blocker($loop);

$value = $blocker->awaitOne($promise);

Suggested API (prototype):

$promise = React\Promise\resolve(123);

$value = Clue\React\Block\awaitOne($promise, $loop);
@clue
Copy link
Owner Author

clue commented Jul 2, 2015

Some random thoughts:

  • The Promise-API already follows a functional pattern, so this feels natural
  • A similar functional API worked out quite well in a related project: clue/promise-timeout.
  • The $loop argument could eventually become obsolete (i.e. optional) once React singularity is being considered.
  • DI for the Blocker is hardly useful, so I'm not really seeing any benefit whatsoever

@clue
Copy link
Owner Author

clue commented Jul 2, 2015

Also, regarding functional composition: Quite a bit of the current API can probably be replaced with a somewhat simpler functional approach.

For example, the awaitAll($promises) method (currently 50+ lines) could be expressed like this:

function awaitAll(array $promises, LoopInterface $loop)
{
    return awaitOne(Promise\all($promises), $loop);
}

(Though this skips cancellation for now, see also #4)

To be clear: I'm not saying this can not be done with our current OOP API, but the functional API certainly makes this more obvious.

@clue
Copy link
Owner Author

clue commented Jul 2, 2015

To expand on the previous comment, we might also consider if we should reduce our API to a single await() method can leave the remaining functional to the user instead.

await($promise, $loop);

It's sufficiently easy to call the following functions yourself:

awaitAll():

await(Promise\all($promises), $loop);

awaitRace() (see also #6 for some background):

await(Promise\race($promises), $loop);

wait() (see also https://github.com/clue/php-promise-timeout):

await(Timeout\resolve($time, $loop), $loop);

@clue
Copy link
Owner Author

clue commented Jul 2, 2015

Also, combined with https://github.com/clue/php-promise-timeout, it's pretty easy to add timeout support to any of the above (see also #2):

await(Timeout\await($promise, $time, $loop), $loop);

Arguably, the $loop argument makes this a bit hard to read. Once "singularity" landed (see also first comment), the resulting call might look like this:

await(Timeout\await($promise, $time));

Similarly, this also applies to slightly more complicated combinations:

await(Timeout\await(Promise\all($promises), $time));

@clue clue added this to the v0.3.0 milestone Jul 5, 2015
@clue clue mentioned this issue Jul 6, 2015
@clue clue closed this as completed in #13 Jul 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant