Consider the following problem: I should turn-off all the lamps in a room. The room is contained in a Set rooms. Is it discouraged to use forEach and if? (I read all my lecture notes nowhere did they mention the .forEach(p -> {if… construct.. However I found it the simplest. If it is discouraged how can I solve the problem otherwise? And why is it discouraged?
public void turnOffLampsInRooms(Set<Room> rooms) {
Set<SmartLamp> set = new HashSet<>(lamps);
set.stream()
.forEach(p -> {if (p.getRoom() != null && rooms.contains(p.getRoom())) {
p.turnOff();
}
});
}
>Solution :
The fact is, you use well optimized Stream API with a bit slower if conditions. Its syntax is correct, it works as should, but it doesn’t look nice and for huge amount of data is slower.
Firstly you should move the if statement to .filter stream call:
public void turnOffLampsInRooms(Set<Room> rooms) {
Set<SmartLamp> set = new HashSet<>(lamps);
set.stream()
.filter(p -> p.getRoom() != null && rooms.contains(p.getRoom()))
.forEach(p -> p.turnOff()); // conditions checked already
}
Next, the .filter can be split into separated ones:
public void turnOffLampsInRooms(Set<Room> rooms) {
Set<SmartLamp> set = new HashSet<>(lamps);
set.stream()
.filter(p -> p.getRoom() != null)
.filter(p -> rooms.contains(p.getRoom())) // the same meaning
.forEach(p -> p.turnOff());
}
And finally change the call to method reference:
public void turnOffLampsInRooms(Set<Room> rooms) {
Set<SmartLamp> set = new HashSet<>(lamps);
set.stream()
.filter(p -> p.getRoom() != null)
.filter(p -> rooms.contains(p.getRoom()))
.forEach(SmartLamp::turnOff); // method reference
}
The main advantage is stream optimization, but it also is far easier to read step-by-step what does the streaming do.