rondagostino commented on PR #13280: URL: https://github.com/apache/kafka/pull/13280#issuecomment-1500522402
> Probably the bigger issue is the extra complexity of implementing our own Set / Map / TreeMap / etc. It's all doable (and perhaps we'll end up doing it for Vavr) but it feels kind of bad when the library has already done that work. i think you are referring to having to implement that if we don't go with the current PR implementation and instead go with something like what you proposed -- since the current PR wrapper implementations don't implement the Set/Map interfaces. Is that correct? In other words, I think you are arguing against your proposal -- I just want to be sure. > handle the different types in an appropriate fashion > `java.util.HashMap`... implement `afterAdding` by copying it with the requested addition I think it is important for people to be able to easily understand what performance they are going to get from a function. If that's not clear, and we end up giving O(1) performance if the downcast to a persistent collection can occur vs. O(N) performance if it cannot -- I think that would make it more likely for O(N) performance to occur inadvertently. Especially since people won't necessarily be able to know what type they have just by reading the immediate code -- if all they know is that have a `java.util.Map` then they might have to trace back to figure out where they actually got that, what the actual type is, and therefore what O() behavior they are going to get if they try to add something to it. > have a bunch of static methods in org.apache.kafka.server.persistent.PersistentCollection like PersistentCollection. afterAdding(K k, V v) and implement them appropriately for all persistent collection types we support. I'm not following this suggestion. I don't think this would give us type safety. It feels like it is just the wrapper class I've implemented here. I'm perhaps missing something. Overall, I'm feeling like type safety is not worth giving up for the very small benefit of eliminating the wrapper class. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
