Comparing or storing buffers is dangerous, for a few reasons.
StringBuffer
is unnecessarily threadsafe, and essentially deprecated in favor of StringBuilder
. Use that class instead.
- Both
StringBuffer
and StringBuilder
are mutable, meaning they cannot be safely inserted into a TreeSet
, as they could be changed (for instance, sb.insert(0, 'z')
would make the string start with z
, changing the result of any successive comparisons) and therefore break the TreeSet
.
- There's little benefit to storing such an object, it's trivial to construct a new
StringBuilder
later if you need to keep using it.
- Your comparator is slow, it needs to reconstruct the string every time the comparator is called, which is
O(m log n)
where m
is the buffer length and n
is the tree size.
I would strongly suggest simply storing the strings in your TreeSet
directly instead. This will work cleaner, be faster, and avoid dangerous edge cases like rendering the TreeSet
out of order.
Here's an example that demonstrates how using mutable objects like buffers in a TreeSet
breaks all expectations:
public static void main(String[] args) {
TreeSet<StringBuilder> tree = new TreeSet<>(new Comparator<StringBuilder>() {
@Override
public int compare(StringBuilder one, StringBuilder two) {
return one.toString().compareTo(two.toString());
}});
char from = 'a', to = 'm'; // change to adjust map size
char holdChar = 'd'; // change holdChar to adjust error location
StringBuilder hold = null;
for(char c = from; c <= to; c++) {
StringBuilder sb = new StringBuilder().append(c).append(c).append(c);
tree.add(sb);
if(c == holdChar) {
hold = sb;
}
}
System.out.println(tree);
hold.insert(0, to);
for(char c = from; c <= to; c++) {
StringBuilder sb = new StringBuilder().append(c).append(c).append(c);
if(c == holdChar) {
sb.insert(0, to);
}
System.out.println("Tree contains "+sb+(tree.contains(sb) ? "" : " NOPE!!"));
}
System.out.println(tree);
}
In theory, every StringBuilder being tested in the second for loop exists in the map, but the map is no longer able to accurately determine this:
[aaa, bbb, ccc, ddd, eee, fff, ggg, hhh, iii, jjj, kkk, lll, mmm]
Tree contains aaa
Tree contains bbb
Tree contains ccc
Tree contains mddd
Tree contains eee NOPE!!
Tree contains fff NOPE!!
Tree contains ggg NOPE!!
Tree contains hhh NOPE!!
Tree contains iii NOPE!!
Tree contains jjj NOPE!!
Tree contains kkk NOPE!!
Tree contains lll NOPE!!
Tree contains mmm
[aaa, bbb, ccc, mddd, eee, fff, ggg, hhh, iii, jjj, kkk, lll, mmm]
Even worse, due to the underlying tree structure, exactly which fields are or are not correctly found is dependant on the map size and the error point. Play with the from
/to
/holdChar
values, and you'll see arbitrarily different results. Try holdChar = 'f';
for example.