Method API

[kraih]

I'm still reading up on the code, so maybe i'm mistaken, but is there a reason for why methods like Mojo::RabbitMQ::Client::Channel::declare_exchange have to return event emitting objects (Mojo::RabbitMQ::Client::Method)?

  my $exchange = $channel->declare_exchange(
    exchange => 'mojo',
    type => 'fanout'
  );
  $exchange->on(success => sub {
    my ($exchange, $frame) = @_;
    ...
  });
  $exchange->on(error => sub {
    my ($exchange, $err) = @_;
    ...
  });
  $exchange->deliver;

As far as i can see they usually just emit two events (success/error), and those just once. Wouldn't it make sense to use one callback and/or a promise? Seems quite a bit more efficient.

# Callback
$channel->declare_exchange((exchange => 'mojo', type => 'fanout') => sub {
  my ($err, $frame) = @_;
  die $err if $err;
  ...
});

# Promise
$channel->declare_exchange_p(exchange => 'mojo', type => 'fanout')->then(sub {
  my $frame = shift;
  ...
})->catch(sub {
  my $err = shift;
  ...
});

[SPodjasek]

The reason was simple - it was faster & simpler to me to implement it that way, I just had to make this module working ASAP so many things can have bad design decissions.

Most of AMQP methods just return <Method>-Ok, so simple callback/Promise should be sufficient to handle them, it will surely make resulting code more readable.

[SPodjasek]

After a quick recap of AMQP-0-9-1 reference I believe there should be only three EventEmitters:

• Mojo::RabbitMQ::Client
• Mojo::RabbitMQ::Channel
• Mojo::RabbitMQ::Consumer

All other setup, utility & publisher methods can be surely implemented as callback/Promise based.

[kraih]

I guess support for promises would be very easy to add without breaking the existing API.

package Mojo::RabbitMQ::Client::Channel;
...
use Mojo::Promise;
...
sub declare_exchange_p {
  my $self = shift;

  my $promise = Mojo::Promise->new;
  my $exchange = $self->declare_exchange(@_);
  $exchange->on(success => sub {shift; $promise->resolve(@_) });
  $exchange->on(error => sub { shift; $promise->reject(@_) });
  $exchange->deliver;

  return $promise;
}

And the same for all similar methods. Regarding documentation, i tend to document these alternative methods like this.

[SPodjasek]

Just started to implement Promises in 6837904f2ab2d1b830b70a3a0231193494c02b4a

[kraih]

Nice, that will make the API a lot more pleasant to use (once the problem with the missing return $promise; has been resolved).

[SPodjasek]

Thnx, closing