java.lang.IllegalStateException in iterator.remove()

Question

Rocket class contains: canCarry(Item item)>checks if this item can be carried/ carry updates the weight with total weight.

U2 class is child of Rocket contains: currentweight, maxWeight=18 tons Item class contains: name to be shipped & weight.

In the method loadU2 I am trying to access a list of items and adding it into one rocket until maxWeight of that rocket is reached . For example I have 216 tons of items to carry returning a list of 12 ships.

It throws me java.lang.IllegalStateException error in the line iterator.remove(). I do not know how to go about it, but it looks like it is not allowing me to remove the items while iterating.

public ArrayList<Rocket> loadU2(ArrayList<Item> loadItems){
    //list of ships
    ArrayList<Rocket> U2Ships = new ArrayList<Rocket>();
    for(Iterator<Item> iterator = loadItems.iterator(); iterator.hasNext();) {      
        //create a new ship
        Rocket tempShip = new U2();
        Item tempItem = iterator.next();
        //loop over items check if it can be filled then remove the item that was filled.
        while(tempShip.currentWeight<tempShip.weightLimit) {
            if(tempShip.canCarry(tempItem)){
                tempShip.carry(tempItem);
                iterator.remove();
            }           
        }
        U2Ships.add(tempShip);
    }
    return U2Ships;
}   


Exception in thread "main" java.lang.IllegalStateException
    at java.base/java.util.ArrayList$Itr.remove(ArrayList.java:980)
    at Simulation.loadU1(Simulation.java:35)
    at Main.main(Main.java:14)

Simplified example of what the code is doing: Assuming maxWeight for each ship = 11 tons ArrayList loadItems = [3,5,5,8,1,2,3,5] tons

 - Ship[1]=[3,5,1,2]
 - new list to iterate over >> [5,8,3,5]
 - Ship[2]=[5,3]
 - new list to iterate over >> [8,5]
 - Ship[3]=[8]
 - new list to iterate over >> [5]
 - Ship[4]=[5]

You have a `while` loop where you potentially call `iterator.remove()` multiple times. That is not possible, you can call it only once. Once it is "removed" you cannot remove it again, it's already gone. — Progman, May 30 '21 at 14:05

Mykhailo Skliar · Accepted Answer · 2021-05-30T14:42:24.227

Please, rewrite your code by creating new ArrayList, instead of changing the existing list inside its own iterator:

public ArrayList<Rocket> loadU2(ArrayList<Item> loadItems){
    //list of ships
    ArrayList<Rocket> U2Ships = new ArrayList<Rocket>();
    ArrayList<Item> updatedLoadItems = new ArrayList<Item>();
    for(Iterator<Item> iterator = loadItems.iterator(); iterator.hasNext();) {      
        //create a new ship
        Rocket tempShip = new U2();
        Item tempItem = iterator.next();
        //loop over items check if it can be filled then only leave the load item that was not fully filled.
        boolean addLoadItem = true;
        while(tempShip.currentWeight<tempShip.weightLimit) {
            if(tempShip.canCarry(tempItem)){
                tempShip.carry(tempItem);
                addLoadItem = false;
            }         
        }
        if (addLoadItem) {
          updatedLoadItems.add(tempItem);
        };
        U2Ships.add(tempShip);
    }
    loadItems.removeAll();
    loadItems.addAll(updatedLoadItems);
    return U2Ships;
}

This is not the best solution, but to provide a better solution, you need to change the signature of public ArrayList<Rocket> loadU2(ArrayList<Item> loadItems)

You can try to improve your code by refactoring it.

Hint: right now your loadU2 method tries to do both things at the same time: change loadItems and create U2Ships. This is a direct violation of the single responsibility principle. Try to imagine the soldier who would try to shoot the gun and throw grenade at the same time! One thing at the time.

Hoopje · Answer 2 · 2021-05-30T15:07:47.123

The problem is here:

while(tempShip.currentWeight<tempShip.weightLimit) {
    if(tempShip.canCarry(tempItem)){
        tempShip.carry(tempItem);
        iterator.remove();
    }           
}

You are calling iterator.remove() within a loop. If the condition tempShip.canCarry(tempItem) holds twice, you call iterator.remove() twice, and this is not allowed (the second time, the item is already removed).

I don't know how the method canCarry is implemented, but note that if it is the case that tempShip.currentWeight<tempShip.weightLimit is true, but tempShip.canCarry(tempItem) is false, your loop will run forever.

score 0 · Answer 3 · answered May 30 '21 at 14:22

0

use listIterator instead of Iterator.

ListIterator<Book> iter = books.listIterator();
while(iter.hasNext()){
    if(iter.next().getIsbn().equals(isbn)){
        iter.remove();
    }
}

like used here.

Remove elements from collection while iterating

answered May 30 '21 at 14:22

Abhishek

423
6
12

FSEL · Answer 4 · 2021-05-30T15:36:38.080

public ArrayList<Rocket> loadU2(ArrayList<Item> loadItems){
    //list of ships
    int shipNum=0;
    int itemsloaded=0;
    ArrayList<Rocket> U2Ships = new ArrayList<Rocket>();
    while(!loadItems.isEmpty()) {      
        System.out.println("number of ships created: "+shipNum++);
        //create a new ship
        Rocket tempShip = new U2();
        
        //loop over items check if it can be filled then only leave the load item that was not fully filled.
     
        while(iterator.hasNext()) {  
            Item tempItem = iterator.next();
            if(tempShip.canCarry(tempItem)){
                System.out.println("number of items loaded: "+(itemsloaded++));
                tempShip.carry(tempItem);
                iterator.remove();                                      
             
           } 
        }
        
        U2Ships.add(tempShip);
    }

    return U2Ships;
}

Thank you guys for the help, this should fix 2 problems: infinity, and the iterator.remove().

`iterator.remove()` was no problem, the way you used it was a problem. In fact, your new solution now has a bug *because* you're not using an iterator. Try it out with a list of two items both of which would fit in a single ship, and note how your code creates two ships rather than one. The reason is, that `loadItems.remove(i)` will change the indices of the tail of the list, and therefore skip an item in the next iteration. Using an iterator and `Iterator.remove()` would solve that. — Hoopje, May 30 '21 at 15:06

java.lang.IllegalStateException in iterator.remove()

4 Answers4