Converting a Map into a sorted TreeSet using streams-CodePudding

The code is:

public TreeSet<VehicleTransportation> allVehicleTransportations(){
        
        Set<VehicleTransportation> sortedVehicles = allVehicleTransportation.entrySet().stream()
                .sorted(Comparator.comparing(Map.Entry::getValue.pollutionLevel , Comparator.reverseOrder()))
                .sorted(Comparator.comparing(Map.Entry::getValue.price))
                .map(Map.Entry::getValue)
                .collect(Collectors.toSet());

        return sortedVehicles;
    }

Where the HashMap is

HashMap<String, VehicleTransportation> allVehicleTransportation = new HashMap<String, VehicleTransportation>();

There are 2 problems I run into using this code. First, for some reason Map.Entry::getValue runs into the error : The target type of this expression must be a functional interface I think the problem here is that the comparator cannot compare between two objects of type VehicleTransportation although pollutionLevel and price are both strings.

Second , I cannot quite figure out how to convert the Set into a TreeSet (maybe use (TreeSet<VehicleTransportation>) ?)

CodePudding user response：

A treeset is sorted anyway, so I would say the streams you are using to add to the treeset, even if it worked as you intend, the sorted items would be resorted according to their natural ordering, destroying any sort order that was created by the stream.

Instead, you should specify a sort order on the treeset, and then just addAll(VehicleTransportation.values()) and forget about streams! it will be sorted according to the sort order.

If you want to sort using streams, you'll have to return something other than the treeset (maybe an arrayList) which will not alter the sort order that you have defined.

CodePudding user response：

If you need a TreeSet, there are other collectors for that:

    Set<VehicleTransportation> sortedVehicles = allVehicleTransportation.entrySet().stream()
            .sorted(Comparator.comparing(Map.Entry::getValue.pollutionLevel , Comparator.reverseOrder()))
            .sorted(Comparator.comparing(Map.Entry::getValue.price))
            .map(Map.Entry::getValue)
            .collect(Collectors.toCollection(() -> new TreeSet<VehicleTransportation>()));

And since you don't want to sort using natural comparator, first create a comparator:

Comparator<VehicleTransportation> cmp =  Comparator.comparing(VehicleTransportation::getPollutionLevel()
          .thenComparing(VehicleTransportation::getPrice);

Then use it:

    Set<VehicleTransportation> sortedVehicles = allVehicleTransportation.entrySet().stream()
            .map(Map.Entry::getValue)
            .collect(Collectors.toCollection(() -> new TreeSet<VehicleTransportation>(cmp)));

Do not perform a double sort !

Also, I'm pretty sure that performance on Stream sorted depends on the source and using a TreeSet (like above) might perform better because otherwise the Stream needs to sort which is not as easy as it seems: you may expect buffering of some sort which the TreeSet will do anyway.

CodePudding user response：

For reference, a non-streams version would look like something like this:

Comparator<VehicleTransportation> c = Comparator.comparing(VehicleTransportation::pollutionLevel)
                                                .reversed()
                                                .thenComparing(VehicleTransportation::price);
Set<VehicleTransportation> s = new TreeSet<>(c);
s.addAll(allVehicleTransportation.values());

CodePudding user response：

You are using an incorrect syntax in your method references. On the right side of the operator :: you should provide a method name, and nothing else.

You can substitute this faulty reference Map.Entry::getValue.pollutionLevel with a lambda expression:

entry -> entry.getValue().pollutionLevel()

For more information on method references, have a look at this tutorial.

Sorting will make sense only if you are collecting the elements into a collection that preserves the order and is not sorted by itself, like ArrayList, LinkedHashSet or ArrayDeque. And there's no need to apply sorting twice. You will not get the same result as if you were sorting by two criteria in one go.

But in this case the data should be stored into a sorted collection, therefore sorting operation in the stream pipeline is redundant.

Instead of two separate comparators, you need a comparator that will establish the order of elements first by pollutionLevel and then by price. Which can be defined like that:

Comparator<VehicleTransportation> byLevelAndPrice =
    Comparator.comparing(VehicleTransportation::getPollutionLevel,
                         Comparator.reverseOrder())
        .thenComparing(VehicleTransportation::getPrice);

Fore more information on how to build comparators with Java 8 methods have a look at this tutorial.

In order to create a TreeSet from values of a map, you can invoke values() and then immediately apply collect() by providing Collectors.toCollection() as an argument:

return allVehicleTransportation.values().stream()
    .collect(Collectors.toCollection(() -> new TreeSet<>(byLevelAndPrice)));

Note that Collectors.toSet() will give you a general purpose implementation of the Set interface, which is currently a HashSet.