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 (that was done in #68), 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]
