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

Reply via email to