1

i am trying to sort a list of object that holds string and a double value:

lock.tryLock();
    if (testTagRev.size() > 0)
Collections.sort(testTagRev, documentSampleComperator);
lock.unlock();

documentSampleComperator is a type documentSampleComparer:

class documentSampleComparer implements Comparator<DocumentSample> {
    @Override
    public int compare(DocumentSample x, DocumentSample y) {
        int ans = x.getText().toString().compareTo(y.getText().toString());
        // ans = utils.listToString(x.getText(), ' ').compareTo(utils.listToString(y.getText(),' ')); also didn't work
        if (ans == 0)
            return Integer.compare(x.hashCode(), y.hashCode());
        else return ans;
    }
}

even though the compactor is transitive i still get this exception :

Exception in thread "main" java.lang.IllegalArgumentException: Comparison method     violates its general contract!
    at java.util.TimSort.mergeLo(TimSort.java:747)
    at java.util.TimSort.mergeAt(TimSort.java:483)
at java.util.TimSort.mergeCollapse(TimSort.java:410)
at java.util.TimSort.sort(TimSort.java:214)
at java.util.TimSort.sort(TimSort.java:173)
at java.util.Arrays.sort(Arrays.java:659)
at java.util.Collections.sort(Collections.java:217)
at Trainer.MCobjectStream.<init>(MCobjectStream.java:64)
at Trainer.filterRev.<init>(filterRev.java:64)
at Trainer.Train.main(Train.java:56)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at     sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at com.intellij.rt.execution.application.AppMain.main(AppMain.java:120)

I am using jdk 1.7.0_45 , can you see where is the problem ?

Edit: utils.listToString turn's list of string to a string, added the lock code and comment the line that i was trying to make it work before . also i should mention that the exception only occurred sometimes but i am not using threads .

user1120007
  • 268
  • 3
  • 13
  • 1
    http://stackoverflow.com/questions/11441666/java-error-comparison-method-violates-its-general-contract – Areo Nov 26 '13 at 14:19
  • if (ans == 0) return Integer.compare(x.hashCode(), y.hashCode()); looks really suspect here. – tjg184 Nov 26 '13 at 14:20
  • 1
    What does the line `utils.listToString(...).compareTo(...)` do? Its result is not used. Are you quoting the whole code? Is this line correct? – Stefan Winkler Nov 26 '13 at 14:22
  • I think "ans" should be -1, 0, +1. Maybe thats the problem? – HectorLector Nov 26 '13 at 14:24
  • @HectorLector, "ans" can be not only -1,0,+1 – Ruslan Ostafiichuk Nov 26 '13 at 14:26
  • What do you mean by "a list of object that _holds_ string and a double value", and "_documentSampleComperator is a type documentSampleComparer_", and "the _compactor_ is transitive" (the Comparator?). Please fix, it will help understand your question. – Bludzee Nov 26 '13 at 14:29
  • @Ruslan Ostafiychuk: Thank you. I am wrong. compareTo returns - "a negative integer, zero, or a positive integer as this object is less than, equal to, or greater than the specified object. " – HectorLector Nov 26 '13 at 14:31
  • http://stackoverflow.com/questions/8327514/comparison-method-violates-its-general-contract – Prabhakaran Ramaswamy Nov 26 '13 at 14:36
  • 1
    Since the comparator does not seem to violate transitivity, I suspect that the compared properties (either `getText().toString()` or `hashCode()` or both) change during the execution. – Holger Nov 26 '13 at 14:45
  • Yes, your Comparator is transitive. Can you show us 'DocumentSample.hashCode()'? – 卢声远 Shengyuan Lu Nov 26 '13 at 14:45
  • to Areo,Prabhakaran the link isn't relevant, my method is transit correctly . to tjg184 and Stefan Winkler the line was there for debugging , it turn the string list to a string, i tried compare that instead of toString(), didn't help. to HectorLector i return only -1 +1 and 0 since i return the result of a compare method of the java libaery. @Holger thank you but i can't see how it's happens, i use lock before calling Collections.sort(testTagRev, documentSampleComperator);. to 卢声远 Shengyuan Lu hashCode() is an java method that come with every object , it's the object address. – user1120007 Nov 27 '13 at 06:41
  • the default implementation of hashcode compares the addresses of the objects and is generally not what you want to use to compare. – bcorso Nov 27 '13 at 07:09
  • You are using `tryLock` but never check whether the locking has been successful. Either use `lock` instead or check the result of `tryLock`. – Holger Nov 27 '13 at 09:30
  • @Holger tried using lock, didn't work also i am not using threads but i am very desperate . – user1120007 Nov 27 '13 at 19:25

2 Answers2

1

Implementing Comparator should not involve the hashCode of the Objects. Usually you just compare each attribute in order of their importance. In your example, if the text is the only thing you want to compare against then it should just be:

Collections.sort(catalog, new Comparator<DocumentSample>() {
    @Override
    public int compare(DocumentSample x, DocumentSample y) {
        return x.getText().toString().compareTo(y.getText().toString());
    }
});

By comparing the default implementation of hashCode (which returns an integer representation of the objects internal address) you are saying that two objects will be considered equal if and only if they are the same object in memory.

bcorso
  • 45,608
  • 10
  • 63
  • 75
  • I added the hashCode after the suggested compactor didn't work with the same exception . Also i have a situation that if two object have the same text i don't want sort to run over one of them. – user1120007 Nov 27 '13 at 07:45
  • Why would you expect sort not to run over one of the objects just because you use hashcode? – bcorso Nov 27 '13 at 16:15
  • since no two object are the seem but they can have the same text. – user1120007 Nov 27 '13 at 19:15
  • If two objects have the same text your code still sorts them. It just sorts them by their random address in memory. So still not sure why you think sort "won't run over one of them" – bcorso Nov 27 '13 at 21:31
  • Because there aren't two object with the same random address , so the object with the same text will be sorted randomly but close to each other . – user1120007 Nov 28 '13 at 10:31
  • So you have a reason for randomly sorting otherwise equal objects? If not then you should remove the code and keep things as simple as possible. – bcorso Nov 28 '13 at 19:22
  • Yes, first of all i put it there in an attempt to solve this exception , and second i need to save all the object even if they have the same text. – user1120007 Dec 02 '13 at 12:10
0

Try to use inner class(without that wierd 2.line in method compare).

 Collections.sort(catalog, new Comparator<DocumentSample>() {
        @Override
        public int compare(DocumentSample x, DocumentSample y) {
            int ans = x.getText().toString().compareTo(y.getText().toString());
            if (ans == 0) {
                return Integer.compare(x.hashCode(), y.hashCode());
            } else {
                return ans;
            }
        }
    }
);