Antti / rust-amqp

AMQP client in pure rust. Corresponds to rabbitmq spec.
MIT License
249 stars 45 forks source link

The bare fn (ConsumerCallback) for consumers is not really useful #10

Closed agentgt closed 9 years ago

agentgt commented 9 years ago

basic_consume takes a bare fn for a callback. The problem with bare functions (opposed to closures and trait objects) is that they apparently cannot contain any environment. This is not very useful.

ie pub type ConsumerCallback = fn(...) seems to be the wrong type design wise to use.

I think a trait object would be the safest bet as a callback especially since many other rabbit libraries including the Java library does this as well. This is because in general there is more than one type of function that needs to be called (you could do it also with a single function and an enum but the other rabbit libraries do not do this).

I could be completely wrong on this as I'm very new to Rust :)

Antti commented 9 years ago

I think you're right. I'm already working on it. Also, I must say I didn't work on ergonomics of this library yet, so any help is appreciated.

Antti commented 9 years ago

@agentgt Have a look now, what do you think? I made basic_consume take T: Consumer, which is also implemented for that ConsumerCallback, which is now called ConsumerCallBackFn. handle_delivery takes &mut self and &mut Channel, which should cover all the cases.

agentgt commented 9 years ago

It looks good. I'll try it today. I was trying to see how it could be done (as I have a similar problem in my own project) and knew about Box but did not know about RefCell... still getting used to Rust lifetimes.

Antti commented 9 years ago

RefCell is workaround for borrow checker, when I'm trying to get item from the consumers: HashMap, it wouldn't allow me to pass the channel into a (retrieved) consumer, because channel is also borrowed (by consumers.get_mut()).

let consumer = channel.consumers.get_mut('consumer_tag'); // <- channel is borrowed mut here.
consumer.handle_delivery(&mut channel, ...);  // <- can't borrow channel again, because it's already borrowed.
petehayes102 commented 8 years ago

Would you be able to give me an example of how you'd implement a closure with an environment? I thought of implementing Fn and Consumer traits on a struct, though Fn traits are currently unstable. Is there a stable alternative?

Antti commented 8 years ago

@petehayes102 Please see examples/consumer.rs I've implemented Consumer for Box<FnMut(&mut Channel, basic::Deliver, BasicProperties, Vec<u8>)>, so you can now declare consumers with a closure.

petehayes102 commented 8 years ago

That looks great! Thanks very much mate.

On 16 Feb 2016, at 10:45 am, Andrii Dmytrenko notifications@github.com wrote:

@petehayes102 https://github.com/petehayes102 Please see examples/consumer.rs I've implemented Consumer for Box<FnMut(&mut Channel, basic::Deliver, BasicProperties, Vec)>, so you can now declare consumers with a closure.

— Reply to this email directly or view it on GitHub https://github.com/Antti/rust-amqp/issues/10#issuecomment-184620645.