Menu
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.
347
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.
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.
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.
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.
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);
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.
36
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With
