keith-turner commented on issue #58: URL: https://github.com/apache/accumulo-access/issues/58#issuecomment-1957683273
> It also has some unnecessary copying (Authorizations.of(Collection<String>) results in two separate calls to Set.copyOf, one of which is redundant). The following is from the Java 11 javadoc for Set.copyOf() ``` If the given Collection is an unmodifiable Set, calling copyOf will generally not create a copy. ``` This means that when something produced by Set.of or Set.copyOf is passed to Set.copyOf it will not make another copy. So doing `Set.copyOf(Set.copyOf(source))` is not a performance issues. It would be ok to remove the redundant calls, but they are not causing a problem. > This may also have implications for hashCode and equals if the implementation stores them in different orderings, but I'm not certain about that. The javadoc for Set clearly defines how equals and hashCode should work for any implementation so that any implementation can be compared to another. So there is no need to have a sorted set for equals and hashcode to work. > It's also just nice to have a sorted view of the authorizations when calling asSet or toString. The to string impl could sort when it constructs the string w/o the rest of the class having to use a sorted set. The sorted set introduces a O(log2(N)) lookup time vs the O(1) for a hashset. IMO it would be best to continue to use what Set.copyOf returns as its an immutable hashset that could be optimized for the exact data since its immutable. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
