@michael: i used void because im used to java beans. thinking about it, i dont see much use for returning false from adding a header: if the headers are in read-only you should probably thrown an IllegalStateException because lets face it, 99% of users dont check return values. returning "this" is probably more useful because it would allow chaining:
Headers.add().add().remove() On Wed, Mar 1, 2017 at 6:47 AM, Ismael Juma <ism...@juma.me.uk> wrote: > Hi Becket, > > Thanks for sharing your thoughts. More inline. > > On Wed, Mar 1, 2017 at 2:54 AM, Becket Qin <becket....@gmail.com> wrote: > > > As you can imagine if the ProducerRecord has a value as a List and the > > Interceptor.onSend() can actually add an element to the List. If the > > producer.send() is called on the same ProducerRecord again, the value > list > > would have one more element than the previously sent ProducerRecord even > > though the ProducerRecord itself is not mutable, right? Same thing can > > apply to any modifiable class type. > > > > The difference is that the user chooses the value type. They are free to > choose a mutable or immutable type. A generic interceptor cannot mutate the > value because it doesn't know the type (and it could be immutable). One > could write an interceptor that checked the type of the value at runtime > and did things based on that. But again, the user who creates the record is > in control. > > From this standpoint allowing headers to be mutable doesn't really weaken > > the mutability we already have. Admittedly a mutable header is kind of > > guiding user towards to change the headers in the existing object instead > > of creating a new one. > > > Yes, with headers, we are providing a model for the user (the user doesn't > get to choose it like with keys and values) and for the interceptors. So, I > think it's not the same. > > > > But I think reusing an object while it is possible > > to be modified by user code is a risk that users themselves are willing > to > > take. And we do not really protect against that. > > > If users want to take that risk, it's fine. But we give them the option to > protect themselves. With mutable headers, there is no option. > > > > But there still seems > > value to allow the users to not pay the overhead of creating tons of > > objects if they do not reuse an object to send it twice, which is > probably > > a more common case. > > > > I think the assumption that there would be tons of objects created is > incorrect (I suggested a solution that would only involve one additional > reference in the `Header` instance). The usability of the immutable API > seemed to be more of an issue. > > In any case, if we do add the `close()` method, then we need to add a note > to the compatibility section stating that once a producer record is sent, > it cannot be sent again as this would cause interceptors that add headers > to fail. > > Ismael >
