I'm trying to work with Maps and lambdas. Firstly I decide to write normal foreach loop and later compare it with lambda and how its shorter. So firstly I have method which display key and values, after that I would like to get sum of this values. Key is object which contain name, price etc., values is count of item.
Below is normal foreach method.
double totalPrice = 0;
int items = 0;
for (Map.Entry<Item, Integer> entry : basketItems.entrySet()) {
System.out.println("Key: " + entry.getKey() +
"Value: " + entry.getValue());
totalPrice += entry.getKey().getPrice() * entry.getValue();
items += entry.getValue();
}
System.out.println("Wartość zamówienia: " + String.format("%.2f", totalPrice));
System.out.println("Ilość przedmiotów: " + items);
This same can I make with lambda like this.
basketItems.entrySet().forEach(
b -> System.out.println("Key: " + b.getKey() +
" Value: " + b.getValue()));
But, how can I get my total price by that way in lambda? Its possible do that in one lambda? I have no idea.
From your question you seem to be under the impression that .forEach is 'better' than for(:) and are now experimenting to prove this.
You can't prove it, because it's false. .forEach is often considerably worse. It's not shorter. Code-quality-wise, it usually results in less elegant code ('elegant' defined as: Easier to understand and follow, easier to modify in the face of change requests, easier to test. Not defined as 'looks prettier to me' - There is no arguing about taste!)
There ARE reasons to use .forEach, but these are generally exotic.
Because lambdas are not transparent in regards to checked exceptions, control flow, and mutable local variables. These are all significant downsides that for(:) doesn't suffer from. The lack of mutable-local-variable-transparency is why you can't write this code, for example.
See below the fold for a full treatise if you are unsure of what these 3 concepts mean and how foreach does it well, lambdas do it badly, and in what scenarios lambdas actually shine and where this lack of transparency turns into upside.
Generally you need to adopt the function mindset. The problem here is that 'transparency of locals' issue. You don't WANT the locals to exist, they inherently mean parallelism isn't on the table (parallelism is rarely relevant, but the architecture of lambdas and streams is fundamentally designed that they should continue to operate efficiently and without race conditions even if it is relevant), and once you write code that fails in parallel scenarios, it tends to grate and cause issues when trying to adopt it into a lambda/stream style.
Only real option is to use an AtomicX to carry the info:
AtomicInteger totalPrice = new AtomicInteger();
AtomicInteger totalItems = new AtomicInteger();
basketItems.entrySet().forEach(b -> {
totalPrice.add(b.getKey().getPrice() * b.getValue());
totalItems.add(b.getValue());
});
This is ugly, inefficient, lots of code, and not particularly readable. Hence why you shouldn't be doing this. This is not an improvement on for(:) whatsoever.
You want each 'loop' to stand alone, not interact with other loops, and then use a parallel approach to combine results. This is called map/reduce, and you should search the web for complete tutorials on this idea. lambdas/streams support the idea too. However, because you're gathering two things, it's considerably more complicated and you need a single object to represent all relevant info: Both the contribution to the total price, as well as the contribution to the item count.
Let's say you JUST want to count items, nothing more. Then don't do:
AtomicInteger count_ = new AtomicInteger();
basketItems.values().forEach(x -> count_.add(x));
int count = count_.get();
but do:
int count = basketItems.values().mapToInt(Integer::intValue).sum();
In your case you're doing two things, so it gets considerably more complicated and hard to read code:
int[] totals = basketItems.entrySet()
.map(e -> new int[] {e.getKey().getPrice() * e.getValue(), e.getValue()})
.collect(Collectors.reducing((a, b) -> new int[] {a[0] + b[0], a[1] + b[1]}))
.orElse(new int[2]);
int totalPrice = totals[0];
int totalItems = totals[1];
This will first map your item/amount pair into an int array where the first elem contains the total price, and the second contains the item count.
It then collects your stream of 2-size int arrays by merging the arrays into a single array.
It then returns this, using [0, 0] as standin in case you have an empty basket.
That's 'thinking in streams'.
Heck, no!
Map.Entry<Item, Integer>), where in the lambda variant you don't. Use var.With that in mind, here is this code in for(:) form:
double totalPrice = 0;
int items = 0;
for (var e : basketItems.entrySet()) {
totalPrice += entry.getKey().getPrice() * entry.getValue();
items += entry.getValue();
}
Simple. Easy to read. Considerably less hacky (no int arrays just to carry info around). Likely an order of magnitude more performant. Pretty much better in every way.
The conclusions drawn in this answer run counter to common if careless advice found in various corners on the internet. (Namely: Do not use lambdas or the functional approach unless the alternative is clearly much worse; when in doubt, don't use lambdas).
Thus, perhaps, one might think the burden of proof lies with this side of the argument (not sure this makes logical sense, but just in case, I guess).
Thus, an in-depth analysis of the 3 transparencies that lambdas do not have and the pros and cons:
try {
Runnable r = () -> { throw new IOException(); }
r.run();
} catch (IOException e) {}
it won't compile, even though your eyeballs and your brain are correctly telling you that it ought to - that IOException is thrown, and caught, guaranteed. It won't compile because your lambda body needs to 'fit' the single abstract method in the Runnable interface which is not declared to allow you to throw IOException, therefore, you can't.
it can be worked around by either aggressively rewrapping checked exceptions in unchecked ones; this ruins the point of checked exceptions and is a ton of boilerplate code to add, making lambdas long and unwieldy.
A basic for loop / don't use lambdas, just doesn't suffer from this, at all:
try {
throw new IOException();
} catch (IOException e) {}
compiles perfectly fine.
int x = 0;
List.of("a", "b").forEach(elem -> x++);
This does not compile: Any non-final local variable cannot be accessed in lambdas within the scope of said local variable (the compiler knows which variable you're talking about, you just can't read or write to it). The compiler will do you a favour and treat any non-final local variable that is nevertheless never changed inside its entire scope as 'effectively final' and let you read from it. However, writing to it from within a lambda is by definition impossible (as that would make it not effectively final). This is annoying. It can be worked around with AtomicInteger / AtomicDouble / AtomicReference (this is better than using new int[1] as vehicle).
for loops do not suffer from this. This compiles fine:
int x;
for (var elem : List.of("a", b")) x++;
outer:
while (true) {
List.of("a", "b").forEach(x -> {
// what goes here?
}
System.out.println("Hello");
}
In the 'what goes here' section, imagine you want to abort not just this particular loop, but the entire forEach run. For example, you want to stop processing when you hit "a", and never even loop for "b". This is impossible. Imagine you want to do something even more drastic break out of the while loop that surrounds it. This is impossible.
Both are quite possible with basic for loops:
outer:
while (true) {
for (var e : List.of("a", "b")) {
if (e.equals("a")) break; // break the for
if (e.equals("b")) break outer; // break the while
}
}
These 3 downsides turn, effectively, into upsides when the lambda 'travels'. That is, the lambda is stored and run at some later time when the method that contains the code of the lambda has long since completed, or if the lambda is executed in an entirely different thread: All 3 downsides I list above become weird and confusing in such scenarios:
That catch block? That entire method has ceased execution, all state is gone, code can't jump there, even though lexically it looks like it should, so that downside turned into an upside.
If non-final locals can be seen and mutated in a lambda that runs in another thread, the local can no longer be declared on stack, it needs to silently be moved to heap. Should we start worrying about marking our locals as volatile now? All options, but right now in java a local is by definition limited to your thread and ceases to exist when your scope ends. That makes it easier to reason about code. These preconditions would have to go away. That's nasty. The downside turned into an upside.
There is no while loop to continue or break to in this scenario. The code would be meaningless. The downside turned into an upside.
This results in the following conclusion:
Lambdas are fantastic, with no caveats, when they are used to write code that 'travels' beyond the scope of where you wrote it, either because it is stored in a field and executed much later, or because it is run in another thread.
Lambdas are dragged down with nasty caveats, thus, bad style and a code smell, if they don't travel. It can still easily be the case that a lambda-based solution is correct even if the lambda does not 'travel', but they aren't just 'better by default'. They are in fact WORSE by default.
Because of the above 2 rules, we get a third rule:
Calling .forEach(elem -> {...}) directly on a list, or on a stream that has no intermediate steps, is ALWAYS bad and a code smell!
In other words, this is guaranteed silly:
list.forEach(x -> doStuff);
set.stream().forEach(x -> doStuff);
Just use basic for loops instead.
The .forEach terminal should therefore rarely be used. It is non-silly code in only 3 scenarios:
The underlying data structure is parallel in nature and the processing you need to do is similarly parallel, but you have no need to finetune the pool used to run it. This is rare (as in, usually if parallelism is relevant like this, you need more control, and fork/join is the answer, OR parallelism doesn't matter. Rare that you're right in the middle), but if you do, then this can help, as these can run in parallel whereas a foreach doesn't. This scenario is very rarely relevant.
You already have a Consumer, e.g. passed in:
public void peekEvents(Consumer<Event> consumer) {
eventQueue.forEach(consumer);
}
eventQueue.stream().filter(Event::isPublic)
.map(Event::getWidget)
.forEach(widget -> System.out.println("Widget with queued up events: " + widget);
Here you filter and map - then it starts to make more sense to use forEach.