Hacker News Reader

Comment by throwaway4good | Hacker News Reader

What you write could be written as:

   ...stream().
     map(e -> e.getKey().getPrice().multiply(new BigDecimal(e.getValue()))).
     reduce(BigDecimal.ZERO, BigDecimal::add)

The problem here is not the use of streams but that the author goes at the problem in a confusing and round about way.

View on HN

That's very pretty to look at and easy to read. But it's a huge pain in the ass to debug when getPrice starts failing. You get a 42000 line stack trace and the only information is that it failed somewhere in that long singular line.

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.

Why would getPrice start failing? If you have side effects or anything other than a trivial implementation inside getPrice, you have more fundamental problems with what you are building than whether to use a for loop or streams.

Why does any code start failing?

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.

First.

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()))).

Is just as dense as the original:

   sum = sum.add(entry.getKey().getPrice().multiply(new BigInteger(entry.getValue())

And the stack trace would be just as confusing.

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.

Could I trouble you for an example or a reference on what this would look like refactored with "Split data and functions, avoid or isolate side-effects"? I would like to understand better but I don't know enough about functional programming to grasp your meaning just from the comment.

I don't know any good resource. And not all functional programmers are good programmers; but try to understand what they are trying to do.

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.

I think definitions vary slightly, but the quality you mentioned (a == b => f(a) == f(b)) would be called a deterministic function [1] - very useful, often applicable or even type check enforced in functional programming languages.

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...

You might read up on pure functions [1] which are unable to produce side-effects when run.

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);
  }

The result of add in doStuff is unused. However, add has a log statement which someone might be relying on elsewhere. The log line makes understanding the usefulness of this code much harder. ie: Can you delete this call? It's impossible to know without understanding everything that might consume the log line. The log-line is a side-effect in these methods.

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.

[1] https://en.wikipedia.org/wiki/Pure_function

> but more difficult to debug.

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.

Because stuff fails?

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.

> 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.

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.

What map returns greatly depends on lambda inside. So yoir collection of beans is changing into collection of good knows what and you have to keep while chaining in mind - because it is nowhere visible.

Whatever exists inside the map lambda would have to exist inside the for loop as well. So if you're dealing with a confusing transformation, a loop doesnt offer you any extra tools for making that transformation more apparent to the reader. Loops have plenty of advantages (computer execution is more obvious, stack traces can be cleaner, easier concept for beginners to grasp), but I have never seen a loop be more readable than a well written functional composition.

Yes and it tends to be more apparent what is its type. It also tends to have a name that helps understanding a lot . It is not even primary loop vs stream difference. This particular frustration is the fluent api vs procedural difference.

It is chaining that obfuscates in this case. Through, I really don't find functional style more readable in general.

The method getPrice is not really correctly named if it did the things you describe. Naming things carefully will give you far more productivity than "unrolled loops, toggleable debug logging and comments on why stuff is done the way it is".

Functions aren't always refactored to their perfect names, for multiple reasons.

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.

   >> ...stream().
       map(e -> e.getKey().getPrice().multiply(new BigDecimal(e.getValue()))).
     reduce(BigDecimal.ZERO, BigDecimal::add)

>That's very pretty to look at and easy to read. But it's a huge pain in the ass to debug when getPrice starts failing.

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)

And the stack trace will tell you if the getKey() or getPrice() failed or the multiply(...) failed.

If someone would kindly explain this to google-java-format, life would be so much better for everyone involved.

intellij IDEA does this formatting very easily, and you can also customize it pretty easily to fit your aesthetics too.

Plus: you can put your formatting rules into your `.editorconfig` and put it under source control.

One downside of this approach is that it only works when you can require everyone who touches the code to use IntelliJ.

Friends don't let friends use Eclipse.

But less flippantly, what other tool is workable for Java?

IntelliJ is my favorite Java IDE, too, but I'm not in the business of telling people what editor to use.

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.

the community edition is free, and usable for most projects that aren't enterprise level.

Looks like it shouldn't when the fluent chain wouldn't fit on one line.

https://github.com/google/google-java-format/issues/341

> you have to do the chain on each line, so the stack track can pinpoint the line it failed at

...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.

I don't think you should be downvoted for this view, but my experience has been pretty good with streams; they took a little getting used to, but now I find they're generally a LOT more robust than the handcrafted stuff. Off-by-ones and NPEs are fairly hard to achieve, and if well done the semantics can be a lot clearer. A lot of enterprise code IS basically plumbing after all!

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.

If it were Python, I could probably write something like:

    sum(key.price * val for (key, val) in cart.items())

But the Java example makes me think "Wait, what?"

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.

Half the problem is that the BigDecimal class is kind of horrid in Java, and so is Map.Entry. The language lacks features that would make them more workable, like operator overloading, implicit constructors and destructuring/pattern matching.

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.

Python's `sum()` works only on numbers and assumes an empty list sums to 0. Java's `reduce()` is quite a bit more flexible.

I use Joda Money and frequently find myself doing:

    final Money bucks = stream.map(Thing::getPrice).reduce(Dollars.ZERO, Money::plus);

If you don't pass in a baseline, you get optional:

    final Optional<Money> bucks = stream.map(Thing::getPrice).reduce(Money::plus);

Also, StreamEx improves the ergonomics of Java streams. The way I would implement the original example:

    EntryStream.of(cart.getProducts())
        .mapKey(Product::getPrice)
        .mapValue(BigDecimal::new)
        .mapKeyValue(BigDecimal::multiply)
        .reduce(BigDecimal.ZERO, BigDecimal::add);

Unfortunately BigDecimal is not especially ergonomic (it could use a `multiply(long)` method, which would eliminate the annoying `mapValue()` above). But unlike the python version of this, it will preserve scale. And streams work as-is on Money types.

What. Python's `sum()` works on anything that defines `__add__`, which can be numbers, strings, lists, or your own custom classes.

  sum([[1, 2], [3, 4]], []) == [1, 2, 3, 4]

Actually it seems sum is special cased to fail for strings, but otherwise you are correct

I dont find 'reduce' to be more readable then + inside loop.

Reply on news.ycombinator.com