ctubbsii commented on code in PR #65:
URL: https://github.com/apache/accumulo-access/pull/65#discussion_r2908833419
##########
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:
The original goal was to have a canonical view of any particular set,
reordering as needed for that view. In other words, if `a1.equals(a2)`, then
`a1.toString().equals(a2.toString())`.
My previous comment suggests that I was abandoning that idea in favor of
just accepting the view of the set that the user passed in, with whatever order
that set originally had, in order to compromise and get a canonical toString
for any authorizations constructed from the same input set, and just avoid the
whole performance concerns about reordering. In other words, have the
toString() view preserve the input order, rather than reorder. That wouldn't
exactly get what I originally wanted, but I probably would have been willing to
compromise on what I wanted with the canonical ordering to get at least an
order-preserving view.
Now, I just don't care anymore, and have given up on the idea of having a
canonical view. I still think it's a nice thing to have, and I really am not
concerned about the performance of toString() here when reordering a tiny set
for printing. toString() shouldn't be used in high-performance code paths
anyway. But, I think it's not worth doing if there's any kind of competing
ideas over that, and am willing to just give up on the idea entirely. As such,
I can close this PR and the original issue.
--
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]
