3

I want to get the minimum value of a function result of an object list. But the return value of this function is optional. So it would be OK if no fragment time is set so far and the return value should then be Optional.empty()

public Optional<Double> getFragmentTime(int fragment) {
    ...
}

private List<Entry> entries; // will be filled in the ctor.

public Optional<Double> getMinFragmentTime(int fragment) {
    return entries.stream()
       .map(e -> e.getFragmentTime(fragment))
       .filter(Optional::isPresent)
       .map(Optional::get)
       .min(Double::compare);
}

Is this the correct way to archive it? The two function calls .filter(Optional.isPresent) and .map(Optional.get) seems rather odd to me, and I think there must be a better solution for it.

Nikolas Charalambidis
  • 40,893
  • 16
  • 117
  • 183
user2622344
  • 956
  • 1
  • 10
  • 26
  • 5
    Well, you could replace `filter(...).map(...)` with `flatMap(Optional::stream)` to get a stream of present optional values only. – Thomas Jan 28 '20 at 14:39
  • thank yo very much, i'm still learning all the tons of different methods of the streams and this seems to be a better solution – user2622344 Jan 28 '20 at 14:44

2 Answers2

7

You can use the advantage of flat-mapping with Optional::stream available since :

return entries.stream()                         // Stream<Entry>
        .map(e -> e.getFragmentTime(fragment))  // Stream<Optional<Double>>
        .flatMap(Optional::stream)              // Stream<Double>
        .min(Double::compare);                  // Optional<Double>

Note that .min(Double.compare); is not correct usage, the parameter is in fact the lambda expression ((d1, d2) -> Double.compare(d1, d2) which shall be shortened as method reference Double::compare. Also using Comparator.comparingDouble(d -> d) is possible.

In case of you have to stick with .filter(Optional::isPresent).map(Optional::get).

Nikolas Charalambidis
  • 40,893
  • 16
  • 117
  • 183
  • Optional::stream means, that only elements which are present will be streamed, and .flatMap() will be used, to reduce the nested Optional's which are returned from stream to only one. Is this correct? – user2622344 Jan 28 '20 at 14:49
  • 2
    It is but the Stream resulting after flatmapping doesn contain only ONE value. The flatmapping flattens the inner structure. Let’s say you have Stream> and with flatmapping you achieve Stream. The same it is woth the optional which holds always up to one item. Optional::stream returns either an empty Stream or a Stream with that one element which assures only the present values remain in the Stream. If you use map instead of flatmap, you’d result with Stream> described above and the subsequent call of self-flatmapping results in Stream like my solution does. – Nikolas Charalambidis Jan 28 '20 at 14:57
  • 1
    @user2622344 Its worth mentioning that `Optional.stream` would require the use of JDK-9 and above. – Naman Jan 28 '20 at 15:30
  • 2
    Why not use a single `.flatMap(e -> e.getFragmentTime(fragment).stream())`. Besides that, I’d use `min(Comparator.naturalOrder())` instead of creating a new comparator. – Holger Jan 28 '20 at 17:01
  • 1) Any benefit of this solution? I always wondered if a sequence of map and flatmap is better than flatmap with a call od stream() inside. 2) Right, I have forgotten to this Comparator. Thanks :) – Nikolas Charalambidis Jan 28 '20 at 21:08
  • 2
    There seems to be a widespread habit of trying to use method references as much as possible. A typical pattern would be `map(Type::collectionReturningMethod) .flatMap(Collection::stream)`. It’s not clear which is more efficient, the differences are tiny anyway. I tend to assume that given the current implementation, a single operation using lambda would win, but, as said, by a tiny margin. When you already have a lambda expression in one of the operations, it’s clear that using two operations can’t win in any way, compared to a single operation using a lambda expression. – Holger Jan 29 '20 at 08:29
  • Thanks Holger, maybe it’s worth to ask a separate question for this to get more oponions and insights from others. I am a bit worried it’s primarly opinion-based. – Nikolas Charalambidis Jan 29 '20 at 09:02
  • 2
    Such questions would need to be written carefully. E.g., avoid anything that reads like “what is better” and emphasize the “what happens under the hood” or “how do they differ technically” and “how can it affect performance” aspect. But note that there’s [an old similar Q&A](https://stackoverflow.com/q/24054773/2711488), so a new question would only be justified if it addresses new aspects, e.g. deeper technical details (and the question would need to be written such that this difference is recognizable). – Holger Jan 30 '20 at 10:19
0

First one should use the stream for the primitive type, as that has a nice support for min().

public OptionalDouble getFragmentTime(int fragment) {
    ...
}

public OptionalDouble getMinFragmentTime(int fragment) {
    return entries.stream()
       .flatMap(e -> e.getFragmentTime(fragment).stream())
       .min();
}

An OptionalDouble may deliver a stream of 1 or 0 doubles. A flatMap to DoubleStream, and then taking the min.

(Code not verified.)

Joop Eggen
  • 107,315
  • 7
  • 83
  • 138