CODEWITHSUNDEEP
javasortingjava-8java-stream
toosensitive
toosensitive

Reputation: 2375

Stream sorting issue

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;
}
                }

Upvotes: 2

Views: 201

Answers (1)

Pshemo
Pshemo

Reputation: 124215

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.

Upvotes: 6

Related Questions