ctubbsii commented on code in PR #65:
URL: https://github.com/apache/accumulo-access/pull/65#discussion_r1683506225
##########
src/main/java/org/apache/accumulo/access/Authorizations.java:
##########
@@ -58,9 +59,12 @@ public int hashCode() {
return authorizations.hashCode();
}
+ /**
+ * @return a String containing the sorted, comma-separated set of
authorizations
+ */
@Override
public String toString() {
- return authorizations.toString();
+ return new TreeSet<>(authorizations).toString();
Review Comment:
I think now that Authorizations.of() takes a set, we could just use the set
that the user passed in, preserving whatever order it already had. If it's an
immutable set already, we can just keep a reference to the same object instead
of doing an unnecessary copy. If it's not, we can preserve the order when we do
the copy (maybe using `LinkedHashSet.addAll()` wrapped with
`Collections.unmodifiableSet`?). The only issue with this approach is that
`asSet()` won't return something that has the `SortedSet` interface if it
originally was a mutable sorted set, like TreeSet, but that might be an
acceptable compromise).
--
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]
