Htydjtk

Given a list of items with properties, I am trying to get the last item to appear with a maximum value of said property.

For example, for the following list of objects:

t  i

A: 3

D: 7 *

F: 4

C: 5

X: 7 *

M: 6

I can get one of the Things with the highest i:

Thing t = items.stream()

        .max(Comparator.comparingLong(Thing::getI))

        .orElse(null);

However, this will get me Thing t = D. Is there a clean and elegant way of getting the last item, i.e. X in this case?

One possible solution is using the reduce function. However, the property is calculated on the fly and it would look more like:

Thing t = items.stream()

        .reduce((left, right) -> {

            long leftValue = valueFunction.apply(left);

            long rightValue = valueFunction.apply(right);

            return leftValue > rightValue ? left : right;

        })

        .orElse(null);

The valueFunction now needs to be called nearly twice as often.

Other obvious roundabout solutions are:

1. Store the object in a Tuple with its index


2. Store the object in a Tuple with its computed value


3. Reverse the list beforehand


4. Don't use Streams

Conceptually, you seem to be possibly looking for something like thenComparing using the index of the elements in the list:

Thing t = items.stream()

        .max(Comparator.comparingLong(Thing::getI).thenComparing(items::indexOf))

        .orElse(null);

To avoid the multiple applications of valueFunction in your reduce solution, simply explicitly calculate the result and put it in a tuple:

Item lastMax = items.stream()

        .map(item -> new AbstractMap.SimpleEntry<Item, Long>(item, valueFunction.apply(item)))

        .reduce((l, r) -> l.getValue() > r.getValue() ? l : r )

        .map(Map.Entry::getKey)

        .orElse(null);

Remove the equals option from the comparator (ie. write your own comparator that doesn't include an equals option):

Thing t = items.stream()

        .max((a, b) -> a.getI() > b.getI() ? 1 : -1)

        .orElse(null);

You can still use the reduction to get this thing done. If t1 is larger, then only it will keep t1. In all the other cases it will keep t2. If either t2 is larger or t1 and t2 are the same, then it will eventually return t2 adhering to your requirement.

Thing t = items.stream().

    reduce((t1, t2) -> t1.getI() > t2.getI() ? t1 : t2)

    .orElse(null);

Stream is not necessary bad if you do things in two steps :

1) Find the i value that has more occurrences in the Iterable (as you did)

2) Search the last element for this i value by starting from the end of items:

Thing t =  

  items.stream()

        .max(Comparator.comparingLong(Thing::getI))

        .mapping(firstMaxThing ->  

                   return

                   IntStream.rangeClosed(1, items.size())

                            .mapToObj(i -> items.get(items.size()-i))

                            .filter(item -> item.getI() == firstMaxThing.getI())

                            .findFirst().get(); 

                            // here get() cannot fail as *max()* returned something.

         )

       .orElse(null)

The valueFunction now needs to be called nearly twice as often.

Note that even when using max, the getI method will be called again and again for every comparison, not just once per element. In your example, it's called 11 times, including 6 times for D, and for longer lists, too, it seems to be called on average twice per element.

How about you just cache the calculated value directly in the Thing instance? If this is not possible, you could use an external Map and use calculateIfAbsent to calculate the value only once for each Thing and then use your approach using reduce.

Map<Thing, Long> cache = new HashMap<>();

Thing x = items.stream()

        .reduce((left, right) -> {

            long leftValue = cache.computeIfAbsent(left, Thing::getI);

            long rightValue = cache.computeIfAbsent(right, Thing::getI);

            return leftValue > rightValue ? left : right;

        })

        .orElse(null);

Or a bit cleaner, calculating all the values beforehand:

Map<Thing, Long> cache = items.stream()

        .collect(Collectors.toMap(x -> x, Thing::getI));

Thing x = items.stream()

        .reduce((left, right) -> cache.get(left) > cache.get(right) ? left : right)

        .orElse(null);