Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How to avoid nested forEach calls?

I have the following code:

interface Device {
    // ...
    boolean isDisconnected();
    void reconnect();
}

interface Gateway {
    // ...
    List<Device> getDevices();
}

...

for (Gateway gateway : gateways) {
    for(Device device : gateway.getDevices()){
        if(device.isDisconnected()){
            device.reconnect();
        }
    }
}

I want to refactor the code using Stream API. My first attempt was like the following:

gateways
    .stream()
    .forEach(
        gateway -> {
            gateway
                .getDevices()
                .parallelStream()
                .filter(device -> device.isDisconnected())
                .forEach(device -> device.reconnect())
            ;
        }
    )
;

I didn't like it so after some modifications I ended up with this code:

gateways
    .parallelStream()
    .map(gateway -> gateway.getDevices().parallelStream())
    .map(stream -> stream.filter(device -> device.isDisconnected()))
    .forEach(stream -> stream.forEach(device -> device.reconnect()))
;

My question is whether there is a way to avoid nested forEach.

like image 347
Hans van Kessels Avatar asked Dec 17 '18 22:12

Hans van Kessels


People also ask

Are nested forEach loops bad?

Nested iterations are not necessarily a bad thing. Even many well-known algorithms rely on them. But you have to be extremely cautious what you execute in the deepest loop, since these commands will be performed very often.

Can you nest forEach loops?

An important feature of foreach is the %:% operator. I call this the nesting operator because it is used to create nested foreach loops. Like the %do% and %dopar% operators, it is a binary operator, but it operates on two foreach objects.

Why streams are better than forEach?

The reason for the different results is that forEach() used directly on the list uses the custom iterator, while stream(). forEach() simply takes elements one by one from the list, ignoring the iterator.

Is stream forEach better than for loop?

forEach() can be implemented to be faster than for-each loop, because the iterable knows the best way to iterate its elements, as opposed to the standard iterator way. So the difference is loop internally or loop externally.


2 Answers

You should flatten the stream of streams using flatMap instead of map:

gateways
    .parallelStream()
    .flatMap(gateway -> gateway.getDevices().parallelStream())
    .filter(device -> device.isDisconnected())
    .forEach(device -> device.reconnect());

I would improve it further by using method references instead of lambda expressions:

gateways
    .parallelStream()
    .map(Gateway::getDevices)
    .flatMap(List::stream)
    .filter(Device::isDisconnected)
    .forEach(Device::reconnect);
like image 55
ETO Avatar answered Sep 19 '22 04:09

ETO


Don't refactor your code into using Streams. You gain no benefits and gain no advantages over doing it like this, since the code is now less readable and less idiomatic for future maintainers.

By not using streams, you avoid nested forEach statements.

Remember: streams are meant to be side-effect free for safer parallelization. forEach by definition introduces side-effects. You lose the benefit of streams and lose readability at the same time, making this less desirable to do at all.

like image 36
Makoto Avatar answered Sep 21 '22 04:09

Makoto