CODEWITHSUNDEEP
javafor-loop
daveH
daveH

Reputation: 3

IndexOutOfBoundsException when using an enhanced for-loop

My enhanced for-loop returns an IndexOutOfBoundsException when I try to run this:

public static ArrayList<Double> multipliser(ArrayList<Double> listen, int multi) {

    for(double elementer : listen) {
        listen.set((int) elementer, listen.get((int) elementer * multi)); 
    }
    return listen;

It works flawlessly with an good old for-loop:

for(int i = 0; i < listen.size(); i++) {
    listen.set(i, listen.get(i) * multi);
}
return listen;

What am I missing?

Upvotes: 0

Views: 158

Answers (3)

Andy Turner
Andy Turner

Reputation: 140319

listen.get((int) elementer * multi)

is not the same as 

listen.get(i)*multi

Similarly in the set.

But an easier way to multiply everything in the list by a constant in-place (in Java 8+) is:

listen.replaceAll(x -> x * multi);

The easiest way pre-Java 8 is to use a ListIterator:

for (ListIterator<Double> it = listen.listIterator(); it.hasNext();) {
  it.set(it.next() * multi);
}

(Note that this is roughly how the default implementation of Collection.replaceAll looks in Java 8+).

Upvotes: 5

daveH
daveH

Reputation: 3

Thank you all very much! I understand now how I was erroneously using the elements and not the index. The code that works (far from optimal) looks like this:

public static ArrayList<Double> multipliser(ArrayList<Double> listen, int multi){
    int i = 0; 
    for(double elementer : listen) {
        listen.set(i, elementer*multi);
        i++;
    }
    return listen;

Thanks everyone and have a great weekend!

Upvotes: 0

Adrien Brunelat
Adrien Brunelat

Reputation: 4642

In your "enhanced for" loop you're setting your elements at the index corresponding to their value instead of the index of the current loop iteration.

Generally speaking I would discourage you to edit a list you're looping on or an object that was passed as an argument. You should create a new list to return instead:

List<Double> result = new ArrayList<>();
for (double elementer : listen) {
    result.add(elementer * multi);
}
return result;

Also use interfaces (List) in method signatures, not implementations (ArrayList):

public static List<Double> multipliser(List<Double> listen, int multi) {

You can even try this instead which is smaller and so 2017+ (Java 8+):

return listen.stream().map(x -> x * multi).collect(Collectors.toList());

Or even replaceAll() as Andy Turner suggested.

Upvotes: 1

Related Questions