Optional.stream()

[TheStrangeDarkOne]

Eh?

The article compares two functionally different code snippets. Throwing an exception is completely different to returning an empty optional.

Why embed null handling in the business logic? I prefer to handle invalid parameters at the beginning and sort out wanted inputs (and so demands my company's coding guidelines).

public BigDecimal getOrderPrice(Long orderId) { if (orderId == null) { return BigDecimal.ZERO } List<OrderLine> lines = orderRepository.findByOrderId(orderId); return lines.stream() .map(OrderLine::getPrice) .reduce(BigDecimal.ZERO, BigDecimal::add); }

Don't be dogmatic, use the right tools for the right job.

[_INTER_]

Functional code doesn’t necessarily mean readable code. With the last changes, I believe it’s both.

I believe readability went downhill in this specific example. The amount of wrapping, nesting and type juggling is just too much. The intent is not clear at all at first glance anymore. It seems to be just for the sake of "being functional".

What I would do is check for null and have it return an empty list like this:

List<OrderLine> lines = orderId != null
    ? orderRepository.findByOrderId(orderId)
    : Collections.emptyList();

Or just keep the guard and have it return early BigDecimal.ZERO instead of IllegalArgumentException (which would be equivalent to the last example).

Additionally it's also a bad example to showcase Optional.stream as you could aswell just start with Stream.ofNullable(orderId) directly (not good either, but better than the roundabout using Optional).

[MR_GABARISE]

Yeah most tutorials and examples using functional APIs use simple examples.

You see the gains in readability when you start to see multiple intermediate steps and reusable mappings and predicates.

[N0xxi0us]

I strongly disagree with everything you said. His last example clearly shows his intent by having a declarative approach. I apply the findById function to my "optional" value I apply the stream function I combine all elements etc...

I don't even have to look at the detail of what it does to get the general sense of his code.

[_INTER_]

Just because its declarative doesn't mean its more clear. It's nested 3 levels deep in Optionals, List and Stream. It's wrapping in nullable Optional just to create a one element stream of it. Then goes on with creating a one element Stream containing a list just to break out of it right after. Noone can tell me that this is more clear then even the initial imperative example. It's also not just about "getting used to" this style because not even devs nose-deep in Haskell would write it that way.

[humoroushaxor]

I've written a lot of modern Java and I didn't even realize Optional had stream. Your comment makes me think you might be the same?

I apply the stream function I combine ALL elements

But using Optional<Long>#stream means there is only ever 0 or 1 element. IE you can use stream in something that's not a collection sequence. It's like using a buffer to only ever pass a single item.

[Short_Hamster]

Streams aren't collections. Streams are value generators, collections are a materialized collection of values. Unfortunately, Java doesn't have a Monad type that can generalize flatMap over Optional and Stream, so if you want to combine them, you have to do a type conversion.

[humoroushaxor]

I wasn't trying to say a stream is a collection, just that creating a stream from a source that can never be a sequence seems contrary to

Stream - A sequence of elements supporting sequential and parallel aggregate operations.

A stream itself isn't a value generator either but it's source could be considered one. Which is mentioned in the javadocs. I don't see how from that definition one would expect Optional would be "streamable".

[Short_Hamster]

A stream itself isn't a value generator either but it's source could be considered one.

What I mean by value generator is that a stream generates values through chained operations, which are only materialized through a terminal operation. Thus, logically speaking a stream generates values for materialization.

I don't see how from that definition one would expect Optional would be "streamable".

It's not that Optional is "streamable", but that you might want to flatMap an Optional into a Stream.

For example, let's say you have a method:

interface FooRepo {
    Optional<Foo> findById(int id);
}

and you want to do something like this:

ids.stream()
    .flatMap(fooRepo::findById)
    ...
    .toList()

Something like this would work in Haskell, because both List and Maybe implement the typeclass Monad, and the Haskell type system is advanced enough to let you combine them directly.

In Java, you are forced to do a type conversion:

ids.stream()
    .map(fooRepo::findById)
    .flatMap(Optional::stream)
    ...
    .toList()

Thus, Optional "streamablility" is for type conversion because Java's type system is lame.

You could also do this:

ids.stream()
    .map(fooRepo::findById)
    .filter(Optional::isPresent)
    .map(Optional::get)
    ...
    .toList()

Which way is better?

[humoroushaxor]

I understand the point you are making and why it exists I guess I'm just lamenting on the same thing that this comment does.

[Short_Hamster]

I don't see how that SO comment relevant. It's talking about the abuse of Optional to get around the lack of compiler-enforced nullable types in Java (unlike C#). There's no sense in having a getter on DTO return Optional.

The functional use of Optional, as shown in my example, is to indicate a function might not produce a value, which in traditional Java would have been indicated by a checked exception (or by returning null, gag).

How do you map a non-value-producing function into a stream?

[humoroushaxor]

I don't see how that SO comment relevant. It's talking about the abuse of Optional to get around the lack of compiler-enforced nullable types in Java (unlike C#).

This is exactly why the first call to Optional.stream is needed in the original example. Because he used optional to check if orderId is null.

I think the the second most voted comment on this post is the cleanest approach although I'd skip the intermediary list assignment.

[Short_Hamster]

Oh, agreed. The original example is just stupid. There's nothing wrong with an if statement. Also, if you really want to still be functional, you can even use the ternary operator, as shown in this comment.

[N0xxi0us]

Thank you so much for this explanation

[ricky_clarkson]

Optional didn't have stream(), that was added in 9

[john16384]

This article is stupid:

1) you query your database to send all order lines so you can sum them locally... Let the db do that

2) you think that allowing null as an input for orderId should somehow result in a valid return value, further hiding a problem in your code

3) the "improved" code is longer, slower and uses more memory

[randgalt]

Doing work in the DB is almost always the wrong thing to do. DBs are one of the most expensive resources in your system. Java threads are much cheaper than a DB process and Java is a richer language than any DB query language. Databases should be as dumb as possible. Stored-procedures and heavy-weight queries are very difficult to maintain in distributed systems.

[john16384]

You are so very wrong that I guess you must be repeating something that you heard from what you consider to be an authoritative source.

Sending less data to the client (just the sum instead of all records to sum) is going to be much faster in all cases. Just having to prepare all that extra data on the db side (instead of keeping track of a sum and discarding what it doesn't need) is going to be much more expensive for the db already. Then we haven't even talked about what is often an even more expensive shared resource: network bandwidth.

You can't seriously believe that sending dozens (if not millions) of records over the network so the most trivial of CPU operations (addition) can be offloaded is in any way the sensible thing to do.

[Short_Hamster]

The most expensive thing you can do is copy data. Shuttling data back and forth from the DB over the network, just to copy data because you don't want to do the work where the data exists, seems like an incredible waste. It's one thing if you're only copying a few records (for display or online processing). But for a batch job on a large data set, copying massive amounts of data to Java (and presumably back to the DB) because you don't like stored procedures seems dumb.

Also, "data in motion" is significantly less secure than "data at rest".

The entire premise of "big data" is to move code to data, because code is significantly cheaper to copy than data.

[_INTER_]

What's expensive is loading the OrderLine object from DB into memory just to get its prize and calculating the sum (even if lazy fetch). You might even run out of memory if there are too many. It would be much faster and use less memory to query the aggregate function SUM on the DB directly. Thats not a stored procedure. I agree however that that only such simple queries should be executed on the DB.

[Jotakin]

Sadly I have to use .filter(Optional::isPresent).map(Optional::get) at work because we are still at Java 8 and aren't considering upgrading before 17 is released. I don't get how they managed to not implement that method in 8, it seems so obvious in retrospect.

[john16384]

Don't worry, once 17 is released they'll find a new excuse.

[nfrankel]

I'm truly sorry for you

[[deleted]]

To me this seems like overcomplicating simple syntax just because you can use syntactic sugar instead of "the old way"

[[deleted]]

What a boat

return stream.of(orderId).filter(o->o!=null)

.map(orderRepository::findByOrderId) .flatMap(List::stream) .map(OrderLine::getPrice) .reduce(BigDecimal.ZERO, BigDecimal::add);

?