2

I defined a custom comparator to sort the name(String) variable of my objects by length.

Here's the code from my person class:

class MyNameLengthCompare implements Comparator<Person> {

        @Override
        public int compare(Person a, Person b) {
            if(a.getName().length() > b.getName().length()) {
                return -1;
            } else if (a.getName().length() < b.getName().length()) {
                return 1;
            } else
                return 0;
        }

    }

Then in my main method I called Collections.sort(personList, new MyNameLengthCompare); and then I added it to my TreeSet myTreeSet.addAll(personList)

But its not sorting by length of name :(

LuxuryMode
  • 33,401
  • 34
  • 117
  • 188
  • I'm not sure either as to why it's not working but you can replace the body of the method with `return a.getName().length() - b.getName().length();` – Bala R May 08 '11 at 04:14
  • 1
    @Bala using 'tricks' like that in a comparator can lead to buffer overflows. In this particular case it happens to be that the length of a String cannot be long enough to overflow, but that practice can lead you to incorrect results in some situations and should be avoided. OP is using a best practice by making it explicitly -1, 0, or 1. – corsiKa May 08 '11 at 04:28

3 Answers3

3

You don't need to sort it before you add it to the tree set. The only thing that matters is whether or not the tree set has the comparator.

corsiKa
  • 81,495
  • 25
  • 153
  • 204
  • Right! Thanks! One question though: how do I create a new instance of my comparator before using it when constructing my TreeSet if I've defined the comparator in a separate class? When I tried to just do MyNameLengthCompare lengthCompare = new MyNameLengthCompare, java complained that "No enclosing instance of type Person is accessible. Must qualify the allocation with an enclosing instance of type Person (e.g. x.new A() where x is an instance of Person)." – LuxuryMode May 08 '11 at 04:22
  • try this: `Comparator myComp = new MyNameLengthCompare();` Then you can use that same one anywhere, like on a blind date, or in a TreeSet constructor. Both work fine. – corsiKa May 08 '11 at 04:26
  • Tried that and still get the same complaint. To be clear, my custom comparator is an inner class in my Person class. Then in a separate class (in main) I tried Comparator lengthCompare = new MyNameLengthCompare(); – LuxuryMode May 08 '11 at 04:30
  • @LuxuryMode either edit in, or send to a pastebin, the entire outer class. I'll try compiling it here and see what comes up. – corsiKa May 08 '11 at 04:32
  • Here's my class with main http://pastebin.com/aLuZjJ1b and here's my Person class http://pastebin.com/Dck0iL9S – LuxuryMode May 08 '11 at 04:35
  • 1
    @Luxury it compiled for me when I changed `Comparator lengthCompare = new MyNameLengthCompare();` to `Comparator lengthCompare = new Person.MyNameLengthCompare();` Notice how I qualify it with `Person.`? Now this may be because I stripped the packaging info at the top (just for simplicity)... – corsiKa May 08 '11 at 04:42
  • Yeah - I was using the default package instead of `levi.com.collections`. – corsiKa May 08 '11 at 05:03
  • 1
    @luxuryMode O..M..G.. I forgot to metion I made another change... And this is probably the one that did it... I made the `MyNameLengthCompare` class a `public static class MyNameLengthCompare` (added `static`)... That's really what fixed it... I just forgot to mention that. :) – corsiKa May 08 '11 at 05:04
  • Crap, now my TreeSet is sorting by length, but it got rid of a bunch of Person objects! Seems like the set is considering Person objects whose name variables are of equal length to be duplicates... – LuxuryMode May 08 '11 at 05:09
  • 1
    @Luxury that is a problem with your comparator. If they end up being the same length, have it return the `compareTo` of the `String` itself, that way it sorts by length, then alphabetically. So instead of `return 0` you end up with `return a.getName().compareTo(b.getName())` – corsiKa May 08 '11 at 06:49
1

Do you construct the TreeSet with the Comparator? If not, the Tree likely ignores your comparator and previous sorting and uses the natural sorting of its contents, that specified by its Comparable compareTo method.

Hovercraft Full Of Eels
  • 283,665
  • 25
  • 256
  • 373
1

Well, I think there is the next issue :

1) Collections.sort are sorting correctly your list.

2) When you add this collection to the TreeSet, it is sorted one more time, and at this time is used Person.compareTo();

3) Try to not use Comparator, try to implement Comparable interface in the Person class and add list to the tree directly, without sorting with Collections.

StKiller
  • 7,631
  • 10
  • 43
  • 56