Sorting a list using Composition of comparator

Question

I have been asked a question in interview where the requirement was to sort a list of Person using JAVA streams and comparator.

Given that, we have list of persons and multiple sort parameters in the input/ Sort parameters will be holding list of attribute name like personFirstName, personLastName and personAge.

I have written following program where I tried to create a composition of Comparator using "thenComparing" method but it did not work.

package com.sample.streams.composition;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.function.Function;
import java.util.stream.Collectors;

public class StreamComposition {

    class Person {
        private String personFirstName;

        private String personAge;

        private String personLastName;

        public Person(String personFirstName, String personAge, String personLastName) {
            super();
            this.personFirstName = personFirstName;
            this.personAge = personAge;
            this.personLastName = personLastName;
        }

        @Override
        public String toString() {
            return "PersonFirstName : [" + personFirstName + "], PersonLastName : [" + personLastName
                    + "], PersonAge : [" + personAge + "]";
        }

        public String getPersonFirstName() {
            return personFirstName;
        }

        public void setPersonFirstName(String personFirstName) {
            this.personFirstName = personFirstName;
        }

        public String getPersonAge() {
            return personAge;
        }

        public void setPersonAge(String personAge) {
            this.personAge = personAge;
        }

        public String getPersonLastName() {
            return personLastName;
        }

        public void setPersonLastName(String personLastName) {
            this.personLastName = personLastName;
        }

    }

    public static void main(String[] args) {
        StreamComposition ob = new StreamComposition();
        Person p1 = ob.new Person("Gunjan", "30", "Shah");
        Person p2 = ob.new Person("Vimal", "36", "Panchal");
        Person p3 = ob.new Person("Pooja", "29", "Wadkar");
        Person p4 = ob.new Person("Pooja", "55", "Thomas");

        List<Person> list = new ArrayList<>();
        list.add(p1);
        list.add(p2);
        list.add(p3);
        list.add(p4);

        System.out.println("------------CASE 1----------------");
        List<String> sortBy = new ArrayList<>();
        sortBy.add("personFirstName");
        sortAndPrint_1(list, sortBy);
        sortAndPrint_2(list, sortBy);

        System.out.println("------------CASE 2----------------");
        sortBy.add("personAge");
        sortAndPrint_1(list, sortBy);
        sortAndPrint_2(list, sortBy);

        System.out.println("------------CASE 3----------------");
        sortBy.add("personLastName");
        sortAndPrint_1(list, sortBy);
        sortAndPrint_2(list, sortBy);


    }


    public static void sortAndPrint_1(List<Person> persons, List<String> sortBy) {
            System.out.println("Method : sortAndPrint_1");
            List sortedPersons = (List) persons
                                        .stream()
                                        .sorted( 
                                                sortBy
                                                .stream()
                                                .map( s -> Comparator.comparing(getFunction(s)))
                                                .reduce((c1,c2) -> c1.thenComparing(c2))
                                                .get())
                                        .collect( Collectors.toList());

            sortedPersons
                .stream()
                .forEach( p -> System.out.println(p));

    }

    public static void sortAndPrint_2(List<Person> persons, List<String> sortBy) {
        System.out.println("Method : sortAndPrint_2");
        sortBy
            .stream()
            .map( s -> Comparator.comparing(getFunction(s)))
            .forEach(c -> Collections.sort(persons,c));


        persons
            .stream()
            .forEach( p -> System.out.println(p));

    }



    public static Function getFunction(String param) {

        Function<Person,String> function = null;
        switch (param) {
        case "personFirstName" :
            function = (p) -> p.getPersonFirstName();
            break;
        case "personLastName" :
            function = (p) -> p.getPersonLastName();
            break;
        case "personAge" :
            function = (p) -> p.getPersonAge();
            break;
        }

        return function;
    }
}

And the output of above program is as below :

------------CASE 1----------------
Method : sortAndPrint_1
PersonFirstName : [Gunjan], PersonLastName : [Shah], PersonAge : [30]
PersonFirstName : [Pooja], PersonLastName : [Wadkar], PersonAge : [29]
PersonFirstName : [Pooja], PersonLastName : [Thomas], PersonAge : [55]
PersonFirstName : [Vimal], PersonLastName : [Panchal], PersonAge : [36]
Method : sortAndPrint_2
PersonFirstName : [Gunjan], PersonLastName : [Shah], PersonAge : [30]
PersonFirstName : [Pooja], PersonLastName : [Wadkar], PersonAge : [29]
PersonFirstName : [Pooja], PersonLastName : [Thomas], PersonAge : [55]
PersonFirstName : [Vimal], PersonLastName : [Panchal], PersonAge : [36]
------------CASE 2----------------
Method : sortAndPrint_1
PersonFirstName : [Gunjan], PersonLastName : [Shah], PersonAge : [30]
PersonFirstName : [Pooja], PersonLastName : [Wadkar], PersonAge : [29]
PersonFirstName : [Pooja], PersonLastName : [Thomas], PersonAge : [55]
PersonFirstName : [Vimal], PersonLastName : [Panchal], PersonAge : [36]
Method : sortAndPrint_2
PersonFirstName : [Pooja], PersonLastName : [Wadkar], PersonAge : [29]
PersonFirstName : [Gunjan], PersonLastName : [Shah], PersonAge : [30]
PersonFirstName : [Vimal], PersonLastName : [Panchal], PersonAge : [36]
PersonFirstName : [Pooja], PersonLastName : [Thomas], PersonAge : [55]
------------CASE 3----------------
Method : sortAndPrint_1
PersonFirstName : [Gunjan], PersonLastName : [Shah], PersonAge : [30]
PersonFirstName : [Pooja], PersonLastName : [Wadkar], PersonAge : [29]
PersonFirstName : [Pooja], PersonLastName : [Thomas], PersonAge : [55]
PersonFirstName : [Vimal], PersonLastName : [Panchal], PersonAge : [36]
Method : sortAndPrint_2
PersonFirstName : [Vimal], PersonLastName : [Panchal], PersonAge : [36]
PersonFirstName : [Gunjan], PersonLastName : [Shah], PersonAge : [30]
PersonFirstName : [Pooja], PersonLastName : [Thomas], PersonAge : [55]
PersonFirstName : [Pooja], PersonLastName : [Wadkar], PersonAge : [29]

Here, I have created two methods sortAndPrint_1 and sortAndPrint_2. In method sortAndPrint_1, I have first created outer stream over person list and then I am trying to apply composition of comparators but it is not working.

So, I wrote another method sortAndPrint_2 where I am iterating over outer stream of sortBy and using Collection.sort method to sort the list. This method is sorting the records as expected after adding missing break statements in switch case.

Here, the aim of the program is to achieve multi level sorting.

I am still facing issues with method "sortAndPrint_1" which generates wrong output. I want the record first sorted on peronFirstName, then on personAge, then on personLastName.

Can someone please suggest me what is wrong with logic of "sortAndPrint_1" ? Is there any problem with the way I have used reduce method ?

Not sure I understand your problem - you have no duplicate first names so sorting by first name and anything else will be exactly equivalent to sorting by first name alone... — Boris the Spider
– Boris the Spider, Commented Aug 4, 2019 at 6:30
@Boris the Spider, Inputs might vary here. We can also consider this example with duplicates names where multi level sorting makes more sense. — Gunjan Shah
– Gunjan Shah, Commented Aug 4, 2019 at 7:18

JB Nizet · Accepted Answer · 2019-08-04 06:29:20Z

2

There are several problems in that code.

The main one is that there is not break after each case in the switch statement, so getFunction() always returns the last one, which returns the age.

Other issues include:

using a String to represent an age (sorting by age will sort by alphabetical order of the string, rather than by numeric order)
using raw types instead of properly-typed generics
the sortAndPrint_2 method which only sorts by the last comparator
not having the required test data in order to test the composition of comparators, since all properties are distinct.

answered Aug 4, 2019 at 6:29

JB Nizet

694k94 gold badges1.3k silver badges1.3k bronze badges

Sign up to request clarification or add additional context in comments.

16 Comments

Gunjan Shah Over a year ago

Thanks for the suggestions. I have added missing break statements and now sortAndPrint_2 method works fine. But sortAndPrint_1 is still not giving correct result.

JB Nizet Over a year ago

No, sortAndPrint_2 can't run fine. It's broken. And regarding sortAndPrint_1, "not giving the correct result" is not relly helpful if you don't say what you expect as a result and what you get as a result.

Boris the Spider Over a year ago

@GunjanShah re sortAndPrint_2 - this is a version of radix sort; it doesn't work because your values aren't "nested". If you wanted sort by first name, then - for each person - by their transaction time, for example, that would work. Radix sort is generally used to sort numbers.

Boris the Spider Over a year ago

@GunjanShah might I suggest you get some paper and a pencil then sort a list of people first only by last name and then only by first name. Are they still sorted by last name after this?

Boris the Spider Over a year ago

@GunjanShah so surely it's clear why sortAndPrint_2 won't work?

|

Boris the Spider · Accepted Answer · 2019-08-05 08:58:28Z

Addressing the points in JB Nizet's great answer

static <T extends Person> Stream<T> sort(final Supplier<Stream<T>> input, final String... properties) {
    return input.get().sorted(compare(properties));
}

static Comparator<Person> compare(final String... properties) {
    return Arrays.stream(properties)
                  .map(TestBench::compare)
                  .reduce(Comparator::thenComparing)
                  .orElseThrow(() -> new IllegalArgumentException("no properties specified"));
}

static Comparator<Person> compare(final String property) {
    switch (property) {
        case "firstName":
            return Comparator.comparing(Person::getFirstName);
        case "secondName":
            return Comparator.comparing(Person::getSecondName);
        case "age":
            return Comparator.comparingInt(Person::getAge);
        default:
            throw new UnsupportedOperationException();
    }
}

static class Person {
    private final String firstName;
    private final String secondName;
    private final int age;

    public Person(final String firstName, final String secondName, final int age) {
        this.firstName = firstName;
        this.secondName = secondName;
        this.age = age;
    }

    public String getFirstName() {
        return firstName;
    }

    public String getSecondName() {
        return secondName;
    }

    public int getAge() {
        return age;
    }

    // toString, equals, hashCode etc
}

Usage

public static void main(final String[] args) throws Exception {
    final List<Person> people = List.of(
            new Person("Bob", "Smith", 29),
            new Person("Bob", "Smith", 30),
            new Person("James", "Smith", 31),
            new Person("Bob", "Jones", 52),
            new Person("James", "Jones", 71),
            new Person("Bob", "Anderson", 61),
            new Person("James", "Anderson", 32)
    );

    sort(people::stream, "firstName", "secondName")
            .forEach(System.out::println);
}

Output

Person{firstName='Bob', secondName='Anderson', age=61}
Person{firstName='Bob', secondName='Jones', age=52}
Person{firstName='Bob', secondName='Smith', age=29}
Person{firstName='Bob', secondName='Smith', age=30}
Person{firstName='James', secondName='Anderson', age=32}
Person{firstName='James', secondName='Jones', age=71}
Person{firstName='James', secondName='Smith', age=31

Personally I would rewrite this to use an enum so that it's typesafe

enum PersonProperty implements Comparator<Person> {
    FIRST_NAME(Comparator.comparing(Person::getFirstName)),
    LAST_NAME(Comparator.comparing(Person::getSecondName)),
    AGE(Comparator.comparingInt(Person::getAge));

    final Comparator<Person> comparator;

    PersonProperty(final Comparator<Person> comparator) {
        this.comparator = comparator;
    }

    @Override
    public int compare(final Person o1, final Person o2) {
        return comparator.compare(o1, o2);
    }

    public Comparator<Person> getComparator() {
        return comparator;
    }

    static Comparator<Person> compare(final PersonProperty... properties) {
        return Arrays.stream(properties)
                .map(PersonProperty::getComparator)
                .reduce(Comparator::thenComparing)
                .orElseThrow(() -> new IllegalArgumentException("no properties specified"));
    }
}

Instead of calling .get() on an Optional unconditionally, I’d use either, .orElseThrow(() -> new IllegalArgumentException("no properties specified")) or providing a fallback, .orElse((a,b) -> 0), which would simply keep the list unchanged when no sort properties are given.

Michiel Leegwater · Accepted Answer · 2019-08-04 06:36:09Z

0

The methods you defined have a List argument. Why? You could also have provided the method reference already. Or even used the API as is. There was no requirement to change the sorting by providing different arguments, right?

My version would also separate sorting and printing for readability and separation of concerns.

List<Person> sorted = list.stream().sorted(Comparator.comparing(Person::getLastName).andThenBy(Person::getFirstName).collect(Collectors.toList());

answered Aug 4, 2019 at 6:36

Michiel Leegwater

1,1904 gold badges12 silver badges27 bronze badges

4 Comments

Gunjan Shah Over a year ago

I was trying to simulate the question which was asked during interview. so, just assume here, we have been given a list of Strings which defines sortBy parameter. And the list size may vary. It may have two params or three params. So we need to create the required comparator at run time depending on list size

Gunjan Shah Over a year ago

Also agree with second points, I should not use String to hold value of age. But I need a common method to provide Functions at runtime. How can I update getFunction method to provide functions with different returns types ?

Gunjan Shah Over a year ago

Also agree with second points, I should not use String to hold value of age. But I need a common method to provide Functions at runtime. How can I update getFunction method to provide functions with different returns types ?

Michiel Leegwater Over a year ago

Then you can use the Function as used by Comparator.comparing, see docs.oracle.com/javase/8/docs/api/java/util/… for the exact generics supplied.

Collectives™ on Stack Overflow

Sorting a list using Composition of comparator

3 Answers 3

16 Comments

1 Comment

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

16 Comments

1 Comment

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related