Why can't I use filter as my last step in a stream
I keep getting told it is bad practice to not terminate a Stream via methods such as collect and findFirst but no real feedback as to why not much said about it in blogs.
It really depends on the context, if you're saying "can I end a stream with an intermediate operation e.g. filter
and not call a terminal operation (an operation that consumes the stream) ever" then yes it's bad practise and kind of pointless because you've just defined some criteria but never asked for the "result".
Streams are lazy in the sense that they don't do anything unless told so by a terminal operation e.g. collect
, findFirst
etc.
If you're saying "is it bad practice to return a stream from a method" then it may be worth reading this answer on whether one should return a stream or a collection.
Further, note that your getBeanViaOptional
logic is operating on an Optional<T>
rather than a Stream<T>
. Yes, they both have map
, flatMap
and filter
but note that an Optional<T>
can only contain one value or it's empty whereas a stream can have one or more.
Your approach of using an Optional instead of the imperative if
s is obviously better in terms of readability, maintenance, etc. so I'd suggest you proceed with that approach, although you can improve it a little bit by using orElseThrow
i.e.:
return Optional.ofNullable(bean)
.map(RequestBean::getFruitBeans)
.map(n -> n.get(0))
.map(FruitBean::getAnotherBeans)
.map(n -> n.get(0))
.map(AnotherBean::getInnerBeans)
.map(n -> n.get(0))
.map(InnerBean::getBeans)
.filter(n -> n.contains("apple"))
.orElseThrow(CustomException::new);
With streams usually none of the intermediate operations will be executed when there is no terminal operation. Your example uses Optional
. Its operations map
and filter
have the same name as some intermediate operations in stream, but they are different. Your example is ok (not bad practice) at the line asked by your question.
Another thing is that (as already pointed out by Aomine) .orElseThrow
is the shorter way to get the value in the Optional
and throw an exception if there is none. Even more important is that it is safer to use .orElseThrow
(or .orElse
if there is a default value). Optional.get()
should be avoided whenever possible. You will get a NoSuchElementException
if there is no value. This is almost as bad as getting a NullPointerException
when not using Optional
. Optional
used in a proper way can protect you from NullPointerException
.