...stream().
map(e -> e.getKey().getPrice().multiply(new BigDecimal(e.getValue()))).
reduce(BigDecimal.ZERO, BigDecimal::add)
The first thing you do is unravel all that crap to a normal loop so you can debug it properly.
And when you're done, you really don't want to go back to the stream way of doing just in case it breaks again and you need to start debugging once more.
>> ...stream().
map(e -> e.getKey().getPrice().multiply(new BigDecimal(e.getValue()))).
reduce(BigDecimal.ZERO, BigDecimal::add)
The trick with java and chaining calls is that you have to do the chain on each line, so the stack track can pinpoint the line it failed at.
so instead of the above, write the code as:
...stream().
map(e -> e.getKey()
.getPrice()
.multiply(new BigDecimal(e.getValue())))
.reduce(BigDecimal.ZERO, BigDecimal::add)
But less flippantly, what other tool is workable for Java?
I can see a company deciding to do that for internal projects. But if it's open source, I'm certainly not interested in creating a situation where it's difficult to comply with a project's coding standards without buying a $500 piece of software.
...this, to me, speaks to a huge tooling failure. The stack trace should have precise information about the region of the source file that the call was embedded in (line+column start+end), as opposed to merely the line.
Where it goes wrong IMO is where a mix of styles and too many inline lambdas splatter a mess of logic into some god-method.
As other comments note, good formatting will help with intelligibility of failures - and IMO sensible logging (at least of error paths) makes it all rather noce to debug.
That’s not the point they were making. The point is that when you start chaining together so many calls, it can become difficult to debug. Prettier to write, but more difficult to debug. But that’s okay, you need to find the balance that’s appropriate for the particular project.
I've found the stream debugger in IntelliJ to be very useful when looking at debugging streams.
https://www.jetbrains.com/help/idea/analyze-java-stream-oper...
The old plugin version of it has the same functionality (and better pictures) - https://plugins.jetbrains.com/plugin/9696-java-stream-debugg... (click the 'more' link)
Selecting a piece of data within the stream shows you how it moved through the stream.
The other part of streams, for me, is that with the additional "each line does one, and only one thing" and you're not putting too much complexity in a single map, I feel that it forces you to write simpler code that doesn't need much debugging. The question of "how did that data get into the stream is where most of the debugging comes from.
If getPrice fails, the stack trace will start there. If it returns null (which it should not) and trigger a npe, then the line:
map(e -> e.getKey().getPrice().multiply(new BigDecimal(e.getValue()))).
sum = sum.add(entry.getKey().getPrice().multiply(new BigInteger(entry.getValue())
Second.
The key thing with streams is to borrow from the functional programming paradigm: Split data and functions, avoid or isolate side-effects.
Do this correctly and there is a quite real plus in productivity.
Imagine you have two methods in Java:
int add(int a, int b) {
log.info("Adding two numbers {} {}", a, b);
return a + b;
}
void doStuff() {
add(1, 2);
add(2, 3);
add(3, 4);
}
In languages that understand "pure functions" there are optimizations that can be done by the toolchain (think automatic memoization, deferred computation, and much more) when only pure functions are called.
There as a bit of zen to it, less is more, in sense that a language gets more powerful if it is more constrained. For example if you know (by the type system or just coding conventions) that p.getPrice() nevers returns null, it is easier to reason about (proof, test, read) the code.
Like wise if you know that if p1 == p2 then p1.getPrice() == p2.getPrice() (that would be no side effects).
If you as some one suggested, need to support some crazy localization, then don't put it into p.getPrice(). If you must, change the name to something telling and make its input explicit: p.calculateLocalizedPrice(locale). Or better make it an explicit function (static method, or maybe something sitting in a service) calculateLocalizedPrice(product, locale) and again have it be without side effects.
Having no side effects is a different very useful quality - function doing exactly as specified and no more (I/O, setting variables ..). I'm not sure if it means not accessing global state, though it is usually better if both inputs and outputs are explicit.
All in, it is usually easier to reason about functions that are explicit, deterministic and side-effect free, yet I find it profoundly more valuable if it can actually be relied on (a known subset of) functions having those qualities.
[1] https://maksimivanov.com/posts/pure-functions-and-side-effec...
Maybe getPrice used to be a static lookup from a map that couldn't fail, but then the Sales Team wanted to go multi-national and now it's a database lookup with multiple dependencies, that can fail.
"But why wasn't it caught in a code review" etc...
Have you actually worked with a big team ever? Why would (or how could) anyone (outside of Google) go through every dependency of the getPrice function and check that every use case is handling errors/exceptions properly?
Stuff breaks, code is read and debugged more often than it's written. Optimising stuff to be easy and fast to write is the wrong way to make maintainable code. Unroll your loops, add toggleable debug logging and add comments why stuff is done the way it is.
getPrice might start up as something that just gets the price, after 5-10 years it might be a complex process accessing some ERP systems.
This is exactly why people like collection functions. For loops can do anything, you have to spend more time reading and understanding the loop to build your mental model of what is happening. Mapping does one thing, transforms a collection into another collection. Same with filter, etc. If you are optimizing for readability, collection functions give way more information to the reader. Your approach is to optimize for debugging, which I'm not saying is wrong, but it's not optimizing for readability.
It is chaining that obfuscates in this case. Through, I really don't find functional style more readable in general.
sum(key.price * val for (key, val) in cart.items())
Personally, the lesson I'm drawing is that your library shouldn't actively fight the native syntax of the language you are using - just use what your language provides, that's the cleanest and easiest way to do it.
But I guess people's tastes are different.
I use Joda Money and frequently find myself doing:
final Money bucks = stream.map(Thing::getPrice).reduce(Dollars.ZERO, Money::plus);
final Optional<Money> bucks = stream.map(Thing::getPrice).reduce(Money::plus);
EntryStream.of(cart.getProducts())
.mapKey(Product::getPrice)
.mapValue(BigDecimal::new)
.mapKeyValue(BigDecimal::multiply)
.reduce(BigDecimal.ZERO, BigDecimal::add);
sum([[1, 2], [3, 4]], []) == [1, 2, 3, 4]Now, the first two are very fair not to want in a language because they allow programmers to bring in an unlimited amount of user-defined complexity.
Some destructuring however, would just make it easier to work with what's already in the language and libraries.
In a more complex example, with multiple steps and potentially multiple streams, using the Streams API allows the algorithmic and business logic parts of the whole calculation to be kept separate, more visible and more maintainable/adaptable as a result. Imperative code in such cases usually involves factoring out chunks of the for-loop into separate functions or classes which IME goes in the opposite direction.
In general, though, streaming abstractions leave that paradigm in the dust once things get become just a tad more complicated.
Using streaming abstractions, you can reason about the flow of your data at a much higher level and create abstractions which would require 100s or 1000s of lines of code to duplicate.
For example, if he decides he needs to buffer the stream, then perform processing on it, then send it out into to the world, he can do all of this super easily by using a few streaming combinators while everything is super clear and type-safe.
I would like to see an example of this, I just don't think streaming libraries, or abstractions in general, can get rid of that much complexity. In most cases, abstractions can't really get rid of complexity, they just move it around.
The issue I have with streaming abstractions is that the implementation tends to be overly complex. For instance, Rx libraries tend to be thousands of lines of code, and they're often written in ways that make it impossible to trace code or understand a call-stack. It makes some things easier, but when you run into a problem it becomes a nightmare to debug.
I'm just not sure it's warranted a majority of the time. Those libraries tend to be huge because they have to cover every single use-case, and they work at a very high level of abstraction. When you're working with stream-like domains, it's often possible to implement the subset of what you would need of that streaming abstraction with normal code, and in a way which is much easier to understand and debug.
People say this pithy truism all the time, and it's just not true. You're probably just talking about indirection and encapsulation.
Real abstractions by their very nature reduce complexity. You could say "No true Scotsman" but I deal with actual abstractions in Haskell all the time and it only simplifies code.
So maybe Java is just deficient in its abstraction capability, but parametric polymorphism and pure streams sound like a good start to me. If a steam makes you uncomfy because it's doing O(n) things "under the hood" and you "like to know what the computer is doing" that's probably just a personal comprehension issue.
There are some abstractions which objectively reduce complexity: i.e. the C programming language abstracts over assembly, and essentially formalizes a subset of assembly in a way which reduces cognitive load for the programmer. This is a good abstraction, but this is not what streaming libraries do.
> If a steam makes you uncomfy because it's doing O(n) things "under the hood" and you "like to know what the computer is doing" that's probably just a personal comprehension issue.
If you have to reach for "you can only not like this because you don't understand this", it means you don't have a real argument. I have implemented event-stream-like systems before, and I am perfectly aware what they do "under the hood".
I can give you an example of how these libraries go wrong. I was working a few years ago on a project which made heavy use of event streams, and one of my colleagues opened a PR which passed all unit tests locally, but failed when the CI system ran the tests. After about half a day of trying to figure out the problem, we determined that it was because a different scheduler was being used when the tests were run during the CI pipeline, which resulted in some concurrency related issues. What's worse, it didn't fail reliably, but only intermittently, so this made it extremely difficult to track down. This is what happens when you hand over your flow of control to another system: it makes it very difficult to reason about.
Abstractions are good when they reduce boilerplate and provide a shorthand for monotonous work. When abstractions try to be smart or magical and do work for you, it's almost always a bad thing.
Depends on your environment ... and the abstractions.
Haskell has a lot of guarantees that C will never have. These guarantees often mean that making abstractions are a lot less error-prone and potentially a lot more useful. In general, stronger language semantics gives the implementation of the language more leeway to optimize. All of the undefined behavior in C is a pretty good reason to step extremely carefully while doing anything, especially creating abstractions.
> the C programming language abstracts over assembly, and essentially formalizes a subset of assembly in a way which reduces cognitive load for the programmer.
Languages like Haskell attempt to abstract over computation in general; It makes C and Java seem a lot closer together in the spectrum of languages. I'm by no means an expert Haskell'er but just dabbling with it has been illuminating and I've written code in various languages since the 90's.
I hope the GP comment "personal comprehension issue" hasn't thrown you too far off. I think there might be a more tasteful way to express what they were thinking but I can't speak for them.
this happens sometimes due to both java being a PITA to do functional programming with, and also partly due to the initial programmer of the streaming logic not breaking down into composable functions.
And with most people being more familiar with imperative style, it makes the functional style harder to maintain _for them_.
> And with most people being more familiar with imperative style, it makes the functional style harder to maintain for them.
It has nothing to do with functional style. It's perfectly feasible to write functional code with minimal abstraction which is easy to trace and understand. The problem is in handing over your flow of control to an overly complex black box of an abstraction layer.
If you need higher level abstraction, you should use it. If you dont need it, you absolutely should not use it.
As for your second point, you could easily add such functionality to the code with a stateful mapping operation that sets the price to 0 for every other item encountered. In your code, you'd have to add another pass of the loop or you'd have to stick the logic for computing the price inside your for loop.
Personally, I've found that decomposing problems into stream based pipelines makes it much easier to decorate additional functionality than imperative code but that's just my personal experience.
public PriceAndRows getPriceAndRows(Cart cart) {
DiscountApplier discounts = new DiscountApplier()
return cart.getProducts()
.entrySet()
.stream()
.map(CartRow::new)
.map(cartRow -> discounts.apply(cartRow)) // Stateful discount application logic
.collect(Collectors.teeing(
Collectors.reducing(BigDecimal.ZERO, CartRow::getRowPrice, BigDecimal::add),
Collectors.toList(),
PriceAndRows::new
));
}You probably want to check all 'rebate rules' attached to all products in the cart whenever one is added/removed. An active rebate will then modify the price of the cart item, and the sum method can be stupidly simple.
There's nothing wrong with passing in a function other than `CartRow::getRowPrice`. That's one of the beautiful things about functional programming, you can alter behavior by passing different functions as parameters.
However, as someone who writes ecommerce code that deals with exactly this situation all the time, I can tell you that the question doesn't really make sense. Discounts are usually represented by data in the cart - either a special field, or a negative line item that gets summed along with the others. You don't just mysteriously have a second cheaper item in the cart.
I have encountered dozens of places where streaming API calls have been reworked into imperative by whoever ends up maintaining it just so they can figure out why the hell it is breaking in some unforseen edge case.
My layman's impression was that 'yield return' was largely syntactic sugar around an iterator, rather similar to Java.
Because it's hard to review, too many things are happening on one line.
What if you have to negate multiplication? You're messing that line even more, with streams, you're adding one line that's easy to read, easier to review, track changes.
Add sum *= -1; on the next line if you think the line is already too complex.
Or just do return -sum;
Except that the stream version is harder to debug and read, but more effective on potentially unlimited stream.
The same line in a for-loop can access anything, including things from other lines.
The primary benefit of these kinds of functional chains is that each step is highly constrained in their capabilities, mainly by their function name and access scope, so you can better verify that it really does do what it says on the tin.
A for loop’s content has to be ultimately read thoroughly, because anything goes. Including modifying lists unrelated to the list under question.
My personal issue with these is that it is all harder to reason about, harder to read, harder to debug. And each time there is real issue, I have to unpack these into procedural, fix and then encrypt again.
I dont see less bugs in code since we started using these. Bugs did not dropped.
I’m not sure why that soul would be the case... it’s whatever came out of the previous action? You’re doing list operations...always.
Nested list operations get convoluted fast, so I generally avoid them (break them up into separate iterations) but otherwise. The only thing I can remember complicating things is issues with type conversion, but the IDE usually tells you what’s up.
The loss of print debugging and useful debugger support is a pain but usually resolved by commenting out half the manipulations and printing immediately.
Otherwise IME it’s a lot simpler to see a series of independent operations on the total list, then it is to follow a for loop interacting with one element at a time.
But personally I switch between the two styles freely, depending on which one is “cleaner” so I likely naturally avoid the more convoluted/difficult cases.
Adding new line is simple change with no increased reading complexity. Modifying already complex line to add new operation will only increase complexity, change log of this line, making it all harder. That's all I'm saying, I'd rather read 11 simple lines, than 4 complex lines.
It wont increase reading complexity either. It wont even force me to think about what hidden map parameters. It will all be visible directly.
On the other hand, in Clojure they are totally natural.
In the other code, you could substitute parallelStream for stream and have it execute in parallel for "free".
1. The intention of the code is obscured by more layers of abstraction, increasing complexity.
2. Small changes to what the code is conceptually doing tend to lead to larger changes to the actual code than with imperative code.
For the first point, reading through this post and the previous post linked, the code is doing the following:
Taking the entries of a map of product to count. Turning those entries into a different class, a row of each product and count. Then it collects this list on one hand. On the other hand, it sums the total, based on the per product cost and the count of the product in the cart. Then it combines the row list and the total cost into one object, returning it.
Deconstructed, half of this code is just unnecessary complexity. A map of products to count, and a list of unique product/count tuples are theoretically identical. You can iterate over them, find specific products, etc. There /might/ be a reason to specifically desire a list; However why would that code be coupled with code to sum the cost of the cart?
All in all, why is the code not just:
For the second point, briefly: Consider how code would have to change to calculate a deal, such as Buy One Get One Free. Such code to do this calculation would add another layer to the collector with another function defined somewhere (or hidden in some other existing abstraction, such as OP's CartRow::getRowPrice) instead of just visible in the function that calculates a cart subtotal. If the deal relied upon concepts not limited to one row at a time, eg buy any two flavors of chips for %25 off, the proposed solution would have to be completely rewritten.