0

I've got a method called removeSup which is supposed to remove an object Supplement from a list of supplements. this is the code for the method:

private static void removeSup(Supplement supToRemove, List<Supplement> listToRemoveFrom) {
   Iterator<Supplement> iterator = listToRemoveFrom.iterator();
                while(iterator.hasNext()){
                    if(iterator.next().equals(supToRemove)){
                        iterator.remove();
                    }
                }
}

there is a class called magazine which defines the list of supplements.

public class Magazine {
  private List<Supplement> supList;
  public List<Supplement> getSupList() {
        return this.supList;
    }
  public void setSupList(List<Supplement> supList) {


      this.supList = supList;
        }
public Magazine(Double cost, String _name){
        this.supList = new ArrayList<>();
        this.weekCost = cost;
        this.name = _name;
    }
    }

the class supplement has the following constructor

public Supplement(String _name, Double _price, String _magName ){
        this.name=_name;
        this.price=_price;
        this.magName = _magName;
    }

in the main class client there is a search that the user can do to remove a certain Supplement

private static void searchSup(){
   System.out.println("Search for Supplement");
        String search = scanner.nextLine();
        for (Supplement sup : magazine.getSupList()) {
            if (!sup.getSupName().equalsIgnoreCase(search)) {
         //do something
        }
        else{
              removeSup(sup,magazine.getSupList());
        }
    }

} the main method in the client class is as follows:

 private Magazine magazine;
        public static void main(String[] args) {
                magazine = new Magazine(3.0, "pop");
                List<Supplement> startList = new ArrayList<>();
            startList.add(new Supplement("Nat Geo", 3.0,"pop"));
            startList.add(new Supplement("Discovery", 5.0,"pop"));
            startList.add(new Supplement("Health", 6.3,"pop"));
            startList.add(new Supplement("IT", 8.3,"pop"));
            magazine.setSupList(startList);
            searchSup();
        }

When I run this program and type any of the added supplements, i get an error

Exception in thread "main" java.util.ConcurrentModificationException
    at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:859)
    at java.util.ArrayList$Itr.next(ArrayList.java:831)
    at Client.searchSup(Client.java:131)
    at Client.searchSup(Client.java:140)
    at Client.main(Client.java:588)

is it the for loop i am using to search giving me an error? if so how would i go about fixing this?

Pindo
  • 1,585
  • 5
  • 16
  • 32
  • 1
    did you read the javadoc for ConcurrentModificationException? also, did you search SO for similar problems? – jtahlborn Apr 03 '14 at 01:06
  • 1
    Instead of traversing the list again in `removeSup` with a new iterator, iterate with an explicit iterator in `searchSup` and use that iterator's `remove` in `searchSup`. – user2357112 Apr 03 '14 at 01:07
  • possible duplicate of [ConcurrentModificationException and a HashMap](http://stackoverflow.com/questions/602636/concurrentmodificationexception-and-a-hashmap) – jtahlborn Apr 03 '14 at 01:08
  • @user2357112 has the best answer IMO – Leo Apr 03 '14 at 01:28
  • @user2357112 i tried doing that and getting the same error. `else{ magazine.getSupList().remove(sup); } like that right? – Pindo Apr 03 '14 at 01:39
  • @Pindo: No, that's the list's `remove`. You need the iterator's `remove`. – user2357112 Apr 03 '14 at 01:41
  • @user2357112 could you post a sample code of how i would do that? – Pindo Apr 03 '14 at 01:52

3 Answers3

0

You generally shouldn't modify a Collection while iterating over it. It's fine to modify elements, but you really shouldn't remove something from a Collection while iterating. See here: http://docs.oracle.com/javase/7/docs/api/java/util/ArrayList.html. Also, the Javadoc for ConcurrentModificationException may be helpful.

You might try returning a new list with the Supplement removed:

 private static List<Supplement> removeSup(Supplement supToRemove, List<Supplement> listToRemoveFrom) {
    List<Supplement> filteredSupplements = new ArrayList<Supplement>();
    for(Supplement supplement : listToRemoveFrom) {
       if(!suppplement.equals(supToRemove)){
           filteredSupplements.add(supplement);
       }

    }

    return filteredSupplements;
}
rpmartz
  • 3,759
  • 2
  • 26
  • 36
  • thanks. is there any way i can go around removing an element from the list? – Pindo Apr 03 '14 at 01:10
  • The `Collection` interface has a `remove()` method but you'll need to read the Javadoc for each collection that implements it. – rpmartz Apr 03 '14 at 01:13
0

It seams that the "magazine" is local var in the method of main, not accessible to searchSup.Fix it like private void searchSup(Magazine magazine) { //... } and more details if you can provide, the codes in Line 131 and 140 will be helpful.

0

I figured out that the search i was doing was not working with what i wanted to do so i created a method which returns an integer of the Supplement in the list.

private static int indexOfSup(List<Supplement> supSearchList, String nameOfSup) {
        for (Supplement sup : supSearchList) {
            if (sup.getSupName().equalsIgnoreCase(nameOfSup)) {
                return supSearchList.indexOf(sup);
            }
        }
        return -1;
    }

i then use this integer to remove from the list. a simple List.Remove(index) worked fine

Thanks for all the replies.

Pindo
  • 1,585
  • 5
  • 16
  • 32
  • You're really doing a lot of work that Java could be doing for you. Look up the `Collection` and `List` interfaces, specifically their `remove()` methods as well as how to override `equals()` for an object. – rpmartz Apr 04 '14 at 14:16