Unexpected behavior of Java ArrayList.set() method with multiplication in forEach loop

Question

Setting initial array values (no issue):

import java.util.ArrayList;
public class arraylists {

    public static void main (String[] args) {
        ArrayList<Integer> numbers = new ArrayList<Integer>();
        numbers.add(3);
        numbers.add(1);
        numbers.add(4);
        numbers.add(2);
        System.out.println(numbers.toString());
    }
}

Console output: [3, 1, 4, 2]

Trying forEach addition (no issue):

numbers.forEach(number -> {
    numbers.set(numbers.indexOf(number), number + 10);
});
System.out.println(numbers.toString());

Console output: [13, 11, 14, 12]

Trying forEach multiplication:

numbers.forEach(number -> {
    numbers.set(numbers.indexOf(number), number * 2);
});
System.out.println(numbers.toString());

Console output: [6, 4, 8, 2]

My question is, why does the array after the multiplication loop have value 4 at index 1 and value 2 at index 3? Shouldn't they be 1 * 2 = 2 and 2 * 2 = 4, respectively?

I tried the above code and was expecting after the forEach multiplication to have an array with values [6, 2, 8, 4]. Instead I got [6, 4, 8, 2].

After the second element has been processed, what's the index of element with value 2? — Sotirios Delimanolis, Dec 24 '22 at 18:31
@SotiriosDelimanolis sorry, I wasn't trying to unilaterally revoke your closure, I thought mine would only count as a vote to reopen. — Federico klez Culloca, Dec 24 '22 at 18:33

score 1 · Accepted Answer · answered Dec 24 '22 at 18:37

1

indexOf returns the index of the first occurrence of a value. Let's walk through the loop and see what's going on.

We start with [3, 1, 4, 2]
The first element is 3. Its index is 0, so we get [6, 1, 4, 2]
The second element is 1. Its index is 1, so we get [6, 2, 4, 2]
The third element is 4. Its index is 2, so we get [6, 2, 8, 2]
The fourth element is 2. Its first index is 1, so we get [6, 4, 8, 2]

Not only is this wrong (as you've seen), it also means that for each iteration you need to search the entire list (with indexOf) for a value that you already should have known where it is.

Using a good old for loop would be better suited here:

for (int i = 0; i < numbers.size(); ++i) {
    numbers.set(i, numbers.get(i) * 2);
}

answered Dec 24 '22 at 18:37

Mureinik

297,002
52
306
350

Or `map`, if they want to use streams. – Federico klez Culloca Dec 24 '22 at 18:39
Oh okay this makes sense to me now, I was confused about how that method worked in practice. Thank you for your help. – Sean Dec 24 '22 at 18:40
@FedericoklezCulloca true, but then you'll have to collect the stream into a new list instead of updating the current one. Could be OK in some (most?) cases, but I didn't want to assume that. – Mureinik Dec 24 '22 at 18:40

Alexander Ivanchenko · Answer 2 · 2022-12-24T18:47:23.533

You're getting an incorrect result during multiplication because when the last element is being processed, the list looks like this [6, 2, 8, 2] (the element at index 1 has been changed to 2). Therefore, indexOf() return 1, not 3 (as you probabely expected).

In cases like this when you need to replace each element in the list, you can use Java 8 method List.replaceAll() which is far more convenient since you don't need to dial with indices:

numbers.replaceAll(number -> number * 2);

System.out.println(numbers);

Output:

[6, 2, 8, 4]

Unexpected behavior of Java ArrayList.set() method with multiplication in forEach loop

2 Answers2