0

I am trying to do sum of numbers in a list using stream api reduce method. It gives correct result for smaller numbers. But when I give larger numbers then it returns a negative number which is not correct.

Here is my code

public void sumOfNum(List<Integer> numbers) {
    long result = numbers.stream().reduce(0, (a, b) -> a + b);
    System.out.print(result);
}

Testcase 1(passed)

Input

12345, 2343,4324,2323,24234

Output

45569

Testcase 2(failed)

Input

256741038, 623958417,623958417,714532089 , 714532089

Output

-1361245246
Naman
  • 27,789
  • 26
  • 218
  • 353

2 Answers2

4

You are adding them as Integers and java has limit for Integer value 2147483647. If you pass this max value you will start to count from minimum value Integer.MAX_VALUE + 1 = -2147483648.

3

You are performing an Integer addition with your current code. My assumption is that you are aware of the numeric overflow that could occur and that's the reason for long type of result. Things would be in place once you map the values of the stream to Long before summing them.

long result = numbers.stream()
        .map(Long::valueOf)
        .reduce(0L, Long::sum);

Or simply put:

long result = numbers.stream()
        .mapToLong(val -> val)
        .sum();
Naman
  • 27,789
  • 26
  • 218
  • 353
  • 1
    Which one would be better? `mapToLong(val -> val)` or `mapToLong(Integer::longValue)`. I assume the `val -> val` involves `Integer -> int -> long` whereas the second one would be just `int -> long`? – Gautham M May 30 '21 at 12:43
  • @GauthamM Right, if the comparison is of the above two,`mapToLong` shall be better. To follow up with the two variants shared by you, the second one shall be performing on the same lines of unboxing, but yeah quite possibly the latter (`Integer::longValue)` might benefit from the use of reference. Honestly, could be worth benchmarking and sharing if such a question doesn't exist. – Naman May 30 '21 at 15:42
  • does unboxing happen in the second variant? Because i think the `int value` inside the `Integer` class is cast into `long`. – Gautham M May 30 '21 at 16:15
  • 1
    @Naman Yes I aware of Integer overflow but I am new to this stream API and I totally forget that I have to convert Integer into long before sum. Now it's working fine after converting Integer into long. Thanks. – madhusudhanan s May 30 '21 at 19:40
  • @GauthamM `javap` over the conversions says that an extra instruction is added while trying to convert via `long apply(Integer integer) { return integer; }` instead of `long apply(Integer integer) { return integer.longValue(); }`. But to me, it seems as this might just be optimized by the hotspot since anyway the latter also does `(long) value` at some point... The code for former ```Code: 0: aload_1 1: invokevirtual #42 // Method java/lang/Integer.intValue:()I. 4: i2l. 5: lreturn``` whereas other one has `1: invokevirtual #48 // Method java/lang/Integer.longValue:()J` – Naman May 31 '21 at 04:25
  • @Naman this conversion Integer to long only needed in stream api? because the following code simply works without any conversion: long min = 0; for(int j=0; j – madhusudhanan s May 31 '21 at 05:43
  • @madhusudhanans In a statement consisting of both int and long, the int would be automatically cast to long. (smaller type is automatically cast to larger type in such scenario). But in an integer stream you are adding all the integers first (int = int + int -> no long involved) and casting to long happens only after the sum is found. – Gautham M May 31 '21 at 05:50
  • Great. very helpful. – madhusudhanan s May 31 '21 at 06:12
  • @Naman thanks for the `javap` recommendation. Are you doubting whether the optimization by the hotspot would make both of the codes similar even though the former `val -> val` involves an additional operation of `Integer.intValue()` for each element as mentioned in the disassembled code? – Gautham M May 31 '21 at 06:19
  • 1
    @GauthamM Yes, though my doubt is not exactly what you phrased it. I doubt the step `4: i2l` i.e. int to long conversion is also a part of the `Integer.longValue` implicitly too. Not sufficient time, but surely something I would try to look deeper in soon. (set apart Stream in the question, that would be something above the basics in my opinion) – Naman May 31 '21 at 08:21
  • 1
    @GauthamM there is no difference, as there can’t be a difference. The source is an `Integer` object and the result is a `long`. The only difference is *where* it happens. When you use `mapToLong(Integer::longValue)` or `mapToLong(i -> i.longValue())`, the conversion happens inside the `longValue()` method of class `Integer`. When you use `mapToLong(i -> i)`, the conversion happens in the synthetic method containing the lambda body. When you use `mapToLong(Integer::intValue)`, the conversion happens in the `ToLongFunction` generated at runtime. In all cases, the code path will contain the `i2l` – Holger May 31 '21 at 11:02
  • @Holger yes anyway there would be an `i2l`. But for `mapToLong(i -> i)` won't there be an additional call to `Integer.intValue()` to unbox the `Integer` before casting the `int` to `long` for each `Integer` of the stream ? – Gautham M May 31 '21 at 11:17
  • 1
    @GauthamM yes, but that’s no challenge to any JIT compiler. It always boils down to reading the value field of the `Integer` and converting it to a `long` value. – Holger May 31 '21 at 11:39