search

danieljoos / libkafka-asio

C++ Kafka Client Library using Boost Asio
MIT License
76 stars 40 forks source link

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

Open DEvil0000 opened 9 years ago

DEvil0000 commented 9 years ago

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.

danieljoos commented 9 years ago

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

DEvil0000 commented 9 years ago

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: