18

Here's my code as per now.

List<Cat> cats = petStore.getCatsForSale();

if (!cats.empty) 
    logger.info("Processing for cats: " + cats.size());

for (Cat cat : cats) {
    cat.giveFood();
}

My colleague writes realy nice code using the Java stream API. I tried to rewrite it as one streaming statement, but I got stuck.

petStore.getCatsForSale().stream.forEach(cat -> cat.giveFood)
    .countTheCats().thenDo(logger.info("Total number of cats: " + x)); // Incorrect... is this possible?

How can I do this? Ideally I want a single streaming statement...

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
vikingsteve
  • 38,481
  • 23
  • 112
  • 156

4 Answers4

18

Your current code is much better without a stream and can further be cutshort to:

if (!cats.isEmpty()) {
    logger.info("Processing for cats: " + cats.size());
}
cats.forEach(Cat::giveFood); // Assuming giveFood is a stateless operation
Naman
  • 27,789
  • 26
  • 218
  • 353
  • Hi and thx for answer. Can this be done in a one liner? Of course I can write `"if (!cats.isEmpty()) logger.info("...")` but the purpose of this question is to write it with streams – vikingsteve Feb 05 '19 at 08:43
  • 3
    In my opinion, is this the only acceptable solution. *Don't* do everything with `Stream`s just because a tutorial told you so, just by looking at every answer here it's clear what the intentions of **this** one here is, but for every other? It's just a mess – Lino Feb 05 '19 at 08:59
11

I am not sure why you want to use streams as the current loop solutions works, but you may as well use a Stream<List<Cat>>:

Stream.of(petStore.getCatsForSale())
    .filter(cats -> !cats.isEmpty())
    .flatMap(cats -> {
        logger.info("Processing for cats: " + cats.size());
        return cats.stream();
    })
    .forEach(Cat::giveFood);

Maybe an optimization:

Stream.of(petStore.getCatsForSale())
    .filter(cats -> !cats.isEmpty())
    .peek(cats -> logger.info("Processing for cats: " + cats.size()))
    .flatMap(Collection::stream)
    .forEach(Cat::giveFood);

Or use this other variant:

Stream.of(petStore.getCatsForSale())
    .filter(cats -> !cats.isEmpty())
    .mapToInt(cats -> {
        cats.forEach(Cat::giveFood);
        return cats.size();
    })
    .findAny()
    .ifPresent(count -> logger.info("Processing for cats: " + count));
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Lino
  • 19,604
  • 6
  • 47
  • 65
  • `petStore.getCatsForSale()` is a `List` – Naman Feb 05 '19 at 08:37
  • @nullpointer I know and with this ugly piece of code consisting of Java8 streams I think it works, correct me if I'm wrong of course :) – Lino Feb 05 '19 at 08:39
  • 3
    @CommonMan not really, its empty vs non-empty here. I doubt `Optional` would be useful here much. – Naman Feb 05 '19 at 08:47
  • Thank you sir! Works :) Is there a way to improve the contents of the "flatMap", so it's one line instead of two? I notice it returns "cats.stream()" there, is that necessary? – vikingsteve Feb 05 '19 at 08:48
  • An opinion based tempted No to the latter solution. I feel like its getting clumsy with the stream approach. (Didn't really downvote though.) – Naman Feb 05 '19 at 08:50
  • @vikingsteve see the second variant – Lino Feb 05 '19 at 08:50
  • @vikingsteve Actually, there's **one line solution**. – Oleg Cherednik Feb 05 '19 at 08:51
  • 3
    @nullpointer I don't really like any of the solutions here except the one from you, I think using Streams for everyday logic is just overkill and makes it extremly hard to understand the simplest of logic – Lino Feb 05 '19 at 08:53
  • Having side-effects in non-terminal operations is very ugly. @nullpointer's solution is way much cleaner. – ETO Feb 05 '19 at 08:53
  • Why did you use Stream.of(cats) instead of cats.stream ? – Mohsen Feb 05 '19 at 08:54
  • and maybe an `Optional` version with Java-11 syntax could be `Optional.of(cats) .filter(Predicate.not(List::isEmpty)) .stream() .peek(cat -> logger.info("Processing for cats: " + cats.size())) .flatMap(Collection::stream) .forEach(Cat::giveFood);` – Naman Feb 05 '19 at 08:55
  • 1
    @Spara so that I have a "wrapper"-stream around the `Cat`-list (`Stream>`) which allows me to operate on the whole list at once, but still inside the stream. Using `cats.stream()` though will give you a stream over all cats (`Stream`) – Lino Feb 05 '19 at 08:56
  • @nullpointer I like your oneliner there, thank you. It works too :) – vikingsteve Feb 05 '19 at 09:00
4
cats.stream()
    .peek(Cat::giveFood)
    .findAny().ifPresent(cat -> logger.info("Processing for cats: " + cats.size()));
Oleg Cherednik
  • 17,377
  • 4
  • 21
  • 35
  • 6
    Starting from `java-9` you may leave your cats **hungry** ))). Execution of `peek` is not guaranteed anymore. Please read [javadoc](https://docs.oracle.com/javase/9/docs/api/java/util/stream/Stream.html#peek-java.util.function.Consumer-) for more details. – ETO Feb 05 '19 at 09:00
  • 1
    @ETO Please look at tags. It says **java-8**! – Oleg Cherednik Feb 05 '19 at 09:06
  • 4
    @oleg.cherednik I think ETO wanted to say that this solution may only work for java-8 which makes it not a favorable solution when one wants to migrate the version upwards :) – Lino Feb 05 '19 at 09:11
  • 2
    @oleg.cherednik Yes, I know. Using `peek` for non-debugging purposes is considered harmful anyway. – ETO Feb 05 '19 at 09:11
  • 3
    Moreover, starting from February 2019 there is [no long-term support](https://java.com/en/download/release_notice.jsp) of `java-8` for cemmercial users. So if you write `java-8` code, consider avoiding potential migration bugs. One may not notice that `java-9`'s `peek` is not working as it did before. Thus your code will have a hidden unobvious bug. – ETO Feb 05 '19 at 09:17
  • Execution of `peek()` is not guaranteed in Java 8, either. Certainly not when followed by `findAny()`, which is a short-circuiting operation. I wrote this code in Java 8: https://stackoverflow.com/a/50880849/507761 – Matthew Read Feb 05 '19 at 19:24
  • Stop spamming. I know all these changes from JVM9. This example works under JVM8 – Oleg Cherednik Feb 05 '19 at 19:26
  • 1
    This does _not_ work in Java 8. Run this and count the "meow"s: http://tpcg.io/VWHXlA – Matthew Read Feb 05 '19 at 19:37
2

I agree with @Lino. This is another alternative based on his idea:

List<Cat> cats = petStore.getCatsForSale();

cats.stream().limit(1)
    .flatMap(c -> {
        logger.info("Processing for cats: " + cats.size());
        return cats.stream();
    }).forEach(Cat::giveFood);
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
David Pérez Cabrera
  • 4,960
  • 2
  • 23
  • 37