[GitHub] [tomee] SvetlinZarev edited a comment on issue #485: Use ListOrderedSet for thread context listeners.

[GitBox <gi...@apache.org>]

SvetlinZarev edited a comment on issue #485: Use ListOrderedSet for thread context listeners.
URL: https://github.com/apache/tomee/pull/485#issuecomment-500374962
 
 
   My point was that ListOrderedSet is the same thing as CopyOnWriteArraySet:
   * Both implement Set
   * Yet both preserve the insertion order
   
   But ListOrderedSet:
   * is an additional dependency
   * uses twice the memory compared to CopyOnWriteArraySet
   * generates twice the garbage when it's modified
   
   In the end, functionality wise, it does not matter which of them is being used, as they do the same thing, but CopyOnWriteArraySet is the superior in terms of resource usage and not being an external dependency.
   
   I admit that I didn't check the previous version, but if CopyOnWriteArraySet is not ok, then ListOrderedSet should not be OK as well. 
   
   My guess is that David did not notice that although CopyOnWriteArraySet implements Set, it's actually a CopyOnWriteArrayList underneath and behaves in the very same way, except that it does not accept duplicates. That's because "Set<ThreadContextListener> listeners" from the original change-set is misleading.Using Set as the type, you implicitly add documentation to the field that the order is not important, because sets do not have an order. So IMO using the actual type instead of the interface  would be better in that case, as it makes it clear that the order of the elements is important.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services