CODEWITHSUNDEEP
javaillegalstateexception
FSEL
FSEL

Reputation: 15

java.lang.IllegalStateException in iterator.remove()

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]

Upvotes: 1

Views: 719

Answers (4)

FSEL
FSEL

Reputation: 15

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

Upvotes: 0

Hoopje
Hoopje

Reputation: 12932

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.

Upvotes: 1

Mykhailo Skliar
Mykhailo Skliar

Reputation: 1367

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.

Upvotes: 1

Abhishek
Abhishek

Reputation: 433

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

Upvotes: 0

Related Questions