Stream sorting issue

Question

I have a list of items (MyDetail object) that I want to sort with stream sorting methods. The object has 3 fields: field1, field2, field3. I want to sort by field3 first then field2 then field1, all with reversed order. So I wrote a method sortMyList.

I have a list of unsorted items unSortedDetails as the following: myDetail1: "20180201", false, false myDetail2: "20180101", false, false myDetail3: "20180101", false, true

after sortMyList(unSortedDetails), I expect my result would be myDetail3, myDetail1, myDetail2, but the actual result is myDetail1, myDetail3, myDetail2, Why?

so if I implement Comparable for MyDetail like the following, then it works as expected. this is so strange. I could not figure out why. thanks for any help!

public List<MyDetail> sortMyList(List<MyDetail> unSortedDetails){
 List<MyDetail> myDetails = unSortedDetails
 .stream().sorted(Comparator.comparing(MyDetail::getField11).reversed()
 .thenComparing(MyDetail::getField2).reversed()
 .thenComparing(MyDetail::getField3).reversed())
 .collect(Collectors.toList());
 return myDetails;
 }
 @Setter
 @Getter
 public class MyDetail{
 String field1;
 Boolean field2; 
 Boolean field3; 
 }
 @Setter
 @Getter
 public class MyDetail implement Comparable<MyDetail>{
 String field1;
 Boolean field2; 
 Boolean field3; 
 @Override
 public int compareTo(MyDetail o) {
 if (this == o || this.equals(o)) return 0;
 if (field3) return -1;
 if (o.field3) return 1;
 if (!field3 && !o.field3 && field2) return -1;
 if(!field3 && !o.field3 &&!field2 && o.field2) return 1;
 if(!field3 && !o.field3
 &&!field2 && !o.field2){
 return o.field1.compareTo(field1);
 }
 return 0;
}
 }

• Jacob G.: Your field* fields are of the type String, but you're trying to treat them as boolean conditions in your if-statements.
• toosensitive: oh, Sorry, field3, field2 are boolean, field1 is string, updated

Answer 1

There are few problems with your Comparator. First is that each time you call reversed() you are reversing all previous settings of comparator.

So your comparator represents (see steps in comments, FieldX reduced to Fx)

Comparator.comparing(MyDetail::getField11) // F1 ASC
 .reversed() //~(F1 ASC) 
 // F1 DESC 
 .thenComparing(MyDetail::getField2) // F1 DESC, F2 ASC 
 .reversed() //~(F1 DESC, F2 ASC) 
 // F1 ASC, F2 DESC
 .thenComparing(MyDetail::getField3) // F1 ASC, F2 DESC, F3 ASC
 .reversed() //~(F1 ASC, F2 DESC, F3 ASC)
 // F1 DESC, F2 ASC, F3 DESC

so you end up with order Field1 DESC, Field2 ASC, Field3 DESC.

To specify reversed order for each field do it by passing already reversed comparator of that field like .thenComparing(Comparator.comparing(YourClass::getField).reversed()).

---

Next issue is order of fields used by Comparator. In your question you said:

I want to sort by field3 first then field2 then field1

but your comparator first checks field1, then field2, then field3 (since in that order you added them via .thenComparing).

Your code should be more like

Comparator.comparing(MyDetail::getField13).reversed()
 .thenComparing(Comparator.comparing(MyDetail::getField2).reversed())
 .thenComparing(Comparator.comparing(MyDetail::getField1).reversed())

So you are creating ~(F3 ASC), ~(F2 ASC), ~(F1 ASC) which results in F3 DESC, F2 DESC, F1 DESC.

---

BTW as pointed by Holger you can achieve same effect by

Comparator.comparing(MyDetail::getField3)
 .thenComparing(MyDetail::getField2) 
 .thenComparing(MyDetail::getField1)
 .reversed()

Notice that Comparaor.comparing(FunctionToValue) and thenComparing(FunctionToValue) will create ASCending Comparators for selected Value.

So first 3 lines construct Comparator describing order F3 ASC, F2 ASC, F1 ASC. After reversing it
~(F3 ASC, F2 ASC, F1 ASC) also gives us desired F3 DESC, F2 DESC, F1 DESC.

• toosensitive: thanks a lot! Your explanation is very clear. Love it!
• toosensitive: One more question, so stream sort vs. comparable on class, which way is faster? I would vote the later but I tested and did not see much difference between the two.
• Pshemo: I can't help you now but that sounds like an interesting question. Consider creating separate post for it and include your test. Anyway I probably wouldn't be bothered at first by performance since purpose of Comparable interface is to provide default order for elements, while purpose of Comparator is to let others use any custom made order. We usually don't need to think about performance of default order vs user-defined one, but like I said this may be interesting question for others to answer.
• Holger: You’re overcomplicating things. To get a comparator using reverse order for all properties, just create a comparator for all properties and reverse the end result: Comparator.comparing(MyDetail::getField3) .thenComparing(MyDetail::getField2) .thenComparing(MyDetail::getField1).reversed()
• Pshemo: @Holger True, I focused mainly on explaining problems (misuse of reversed and wrong order of fields). But tbh while approach suggested by you is valid, IMO using thenComparing(reversedComparator) is easier to follow, so also less error-prone. Anyway thanks for pointing it out, will add your variant to the answer.