Functions should require callable instead of closures about amp HOT 10 CLOSED

We chose Closure for a few reasons:

• The callable type in PHP is scope-dependent. What is callable in one scope may not be callable when attempting to invoke it from another scope (Luckily this problem will go away in PHP 9: https://wiki.php.net/rfc/deprecate_partially_supported_callables) (EDIT: I was wrong, that RFC does not completely clean up scope issues, visibility problems can still happen with callable, but cannot with Closure.)
• callable cannot be used as a class property (largely due to the above), so is more cumbersome to use.
• First-class callable syntax makes it easy to create closures. [$this, 'method'] being a "callable" has always been strange IMO. $this->method(...) is far superior, clearly indicating that a closure is being created.
• Using Closure does allow us to widen the type to callable at a later time without BC breaks

Invokable objects unfortunately suffer because of this decision. First-class callable syntax does work with invokable objects. async($object(...)) is clear that the intention is to pass a closure to be called. async($object) may be be confused for passing the object to be manipulated or a mistake if $object were not an invokable object.

I'm willing to consider changing Closure to callable in the future, but for the near term I think I'd like to keep Closure.

from amp.

Fair enough, although I don't see how any of those 2 disadvantages are a factor for util functions. First one is more for authors of callables (me), second one for classes, while amp util functions are not classes. But of course, my position is purely from trying to have slightly nicer syntax. $object->method(...) looks fine in such trivial example, but what about multi line calls? With callables accepted, I could do

with callables accepted, I could do

async(new Responder(
    $logger,
    $bufferer,
    $output,
    $this->customHttpHeaders,
    new ResourceInputStream(fopen($bufferer->filePath, 'rb')),
    $newConnDefer
));

without them, I have to do either

async((new Responder(
    $logger,
    $bufferer,
    $output,
    $this->customHttpHeaders,
    new ResourceInputStream(fopen($bufferer->filePath, 'rb')),
    $newConnDefer
))(...));

or

$responder = new Responder(
    $logger,
    $bufferer,
    $output,
    $this->customHttpHeaders,
    new ResourceInputStream(fopen($bufferer->filePath, 'rb')),
    $newConnDefer
);
async($responder(...));

and I'm not a fan of either. But again, it's something we all can live with, just adding my suggestion for minor improvement.

from amp.

async(fn () => new Responder(
    $logger,
    $bufferer,
    $output,
    $this->customHttpHeaders,
    new ResourceInputStream(fopen($bufferer->filePath, 'rb')),
    $newConnDefer
));

from amp.

Actually, in my cases I also need to pass arguments to Responder so above wouldn't work. with callable support I would be able to do

async(
  new Responder(
    $logger,
    $bufferer,
    $output,
    $this->customHttpHeaders,
    new ResourceInputStream(fopen($bufferer->filePath, 'rb')),
    $newConnDefer
  ),
  $socket,
); 

but with arrow functions it would be

async(
  fn($arg) => (new Responder(
    $logger,
    $bufferer,
    $output,
    $this->customHttpHeaders,
    new ResourceInputStream(fopen($bufferer->filePath, 'rb')),
    $newConnDefer
  ))($arg),
  $socket,
); 

which doesn't bring benefit over

async(
  (new Responder(
    $logger,
    $bufferer,
    $output,
    $this->customHttpHeaders,
    new ResourceInputStream(fopen($bufferer->filePath, 'rb')),
    $newConnDefer
  ))(...),
  $socket,
); 

from amp.

What in the hell are you talking about?

from amp.

https://3v4l.org/dO0Me

from amp.

Yes but that's arguably worse than https://3v4l.org/ZePLZ (desired) or https://3v4l.org/GZ5C8 ((...) syntax)

from amp.

Stop trying to code other people's libraries for them.

from amp.

Well, this doesn't look like it's going to be a productive discussion. Sorry that my fresh feedback from amp 2.x -> 3.x update experience bothered you. I was used to pass callable to asyncCoroutine, but this is no longer accepted. All I'm saying is I would prefer if I could still pass callables, because it's more flexible and has nicer syntax, that's it.

from amp.

I also prefer to use callable, in amp v2, I have lots of code like call([$target, 'wakeup']), but in php8.1, first-class callable syntax is acceptable as an alternative async($target->wakeup(...)).

In your example, a callable object implement __invoke is useful, but it's not commonly used.

class CallableThing {

    public function __invoke()
    {
    }

}

$callableThing = new CallableThing();
$callableThing();

So, for now, I think you should accept first-class callable syntax or use closure fn().

from amp.