how to implement a comparator for StringBuffer class in Java for use in TreeSet?

Question

I want to implement a Comparator for StringBuffer that compares the StringBuffer and adds to TreeSet accordingly. This is purely for learning purposes only. I know having a mutable object in a Hasable collection is a bad idea. but the purpose here is how to implement comparator for existing StringBuffer class of Java and use it to create a TreeSet. My current code is below. The code does not compile. kindly help me out here. Thanks

public class ComparatorExample {


    public class SbufferComparator implements Comparator<StringBuffer> {

        @Override
        public int compare(StringBuffer s1, StringBuffer s2) {
            return s1.toString().compareTo(s2.toString());

        }

    }


        public static void main(String[] args) {
                StringBuffer one = new StringBuffer("one");
                StringBuffer  two = new StringBuffer("two");
                StringBuffer three = new StringBuffer("three");
                Set<StringBuffer> sb=new TreeSet<StringBuffer>(new SbufferComparator());
                sb.add(one);
                sb.add(two);
                sb.add(three);
                System.out.println("set before change: "+ sb);
                one.append("onemore");
                System.out.println("set After change: "+ sb);
            }
        }

Answer 1

The inner class SbufferComparator must be static, or must be refactored to a top-level class.

Otherwise, it needs an enclosing object instance to be constructed:

ComparatorExample ce = new ComparatorExample();
SbufferComparator comparator = ce.new SbufferComparator();

Leaving it as a non-static inner class doesn't make much sense, since it doesn't use any instance field or method from its enclosing class. So make it static or top-level.

Answer 2

You should create the comparator class in a separate file not in the same class or make it static instead if you want to keep it inner.

    import java.util.Comparator;
    import java.util.Set;
    import java.util.TreeSet;

public class ComparatorExample {
private static class SbufferComparator implements Comparator<StringBuffer> {

        @Override
        public int compare(StringBuffer s1, StringBuffer s2) {
            return s1.toString().compareTo(s2.toString());

        }

}


    public static void main(String[] args) {
            StringBuffer one = new StringBuffer("one");
            StringBuffer  two = new StringBuffer("two");
            StringBuffer three = new StringBuffer("three");
            Set<StringBuffer> sb=new TreeSet<StringBuffer>(new SbufferComparator());
            sb.add(one);
            sb.add(two);
            sb.add(three);
            System.out.println("set before change: "+ sb);
            one.append("onemore");
            System.out.println("set After change: "+ sb);
        }
    }

Pay attention to the import statements!

Answer 3

Comparing or storing buffers is dangerous, for a few reasons.

1. StringBuffer is unnecessarily threadsafe, and essentially deprecated in favor of StringBuilder. Use that class instead.
2. 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.
3. 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.
4. 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.

Answer 4

right now you have a inner class called SbufferComparator. Since it is non-static, you need an instance to access its method. you could simply make it static and that work!