libkafka-asio icon indicating copy to clipboard operation
libkafka-asio copied to clipboard

improvement: allow setting a Message `Key` as `AddValue` parameter

Open DEvil0000 opened this issue 10 years ago • 2 comments

It is possible to set the Key for a message on the message and then pass it to AddMessage but there is no way to set it with AddValue. This way you offer two different APIs to do the same but one of them is not supporting all options/features. Maybe there should be only one simple to use API I would prefer the AddValue API. I think the user should not know about the Message class and there are just a few fields to set. And using the message class on the outside means to create and destroy a object just to call it (because you need a internal copy until it is delivered) -> overhead.

DEvil0000 avatar Jul 02 '15 15:07 DEvil0000

I fully agree. This one needs to go away. Probably better to just have AddMessage!?

danieljoos avatar Jul 05 '15 09:07 danieljoos

I don't like the AddMessage method signature.

  1. you have to create a Message, call some methods to set all attributes.
  2. depending on the use case you have to create a MessageSet then.
  3. then you pass the object to the AddMessage method which makes a deep copy of it.
  4. then you have to clean up the MessageSet and the Message
  5. after that the lib will clean up the same information again. this sounds like overhead. calling much more methods then needed. and doing some memory allocations we don't need (2* MessageSet and the Messages). I think it would be better to improve the AddValue method and maybe rename it to AddMessage. We know all components of a Message and they are just a few:
  • topic (may be given by config on produce/producer level)
  • partition (may be given by config on produce/producer level)
  • key (optional)
  • value the rest is calculated or can be set by config

DEvil0000 avatar Jul 06 '15 08:07 DEvil0000