4
    private int checkLevel(String bigWord, Collection<String> dict, MinMax minMax)
{
    /*value initialised to losing*/
    int value = 0; 
    if (minMax == MinMax.MIN) value = 1; 
    else value = -1; 


    boolean go = true;

    Iterator<String> iter = dict.iterator();

    while(iter.hasNext())
    {
        String str = iter.next(); 
        Collection<Integer> inds = naiveStringSearch(bigWord, str);

        if(inds.isEmpty())
        {
            iter.remove();
        }

        for (Integer i : inds)
        {
            MinMax passin = minMax.MIN;
            if (minMax == MinMax.MIN) passin = minMax.MAX;

            int value2 = checkLevel(removeWord(bigWord, str, i), dict, passin); 
            if (value2 == -1 && minMax == minMax.MIN)
            {
                value = -1; 
                go = false;
            }
            if (value2 == 1 && minMax == minMax.MAX)
            {
                value = 1; 
                go = false; 
            }

        }

        if (go == false) break; 
    }


    return value;
}

Error:

Exception in thread "main" java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextEntry(HashMap.java:810)
at java.util.HashMap$KeyIterator.next(HashMap.java:845)
at aStringGame.Main.checkLevel(Main.java:67)
at aStringGame.Main.test(Main.java:117)
at aStringGame.Main.main(Main.java:137)

What's the problem here?

dwjohnston
  • 11,163
  • 32
  • 99
  • 194

4 Answers4

5

Something somewhere is modifying dict. I suspect it might be happening inside this call:

int value2 = checkLevel(removeWord(bigWord, str, i), dict, passin);
                                                     ^^^^

edit Basically, what happens is that the recursive call to checkLevel() modifies dict through another iterator. This makes the outer iterator's fail-fast behaviour to kick in.

NPE
  • 486,780
  • 108
  • 951
  • 1,012
  • ^I've updated the question to show that it's a recursive method. What would the best solution be? Cloning the dictionary I'm passing in? – dwjohnston Dec 03 '12 at 22:16
  • @jahroy - but the collection is a hashset. (The reason for it being a hashset is that I'm concerned about performance, I'm not sure that a hashset will be anyfast for simple iterating, and removing elements but). – dwjohnston Dec 03 '12 at 22:21
  • If you are iterating, all collections are equal in terms of performance. BTW if you use a Concurrent Set you won't have this isssue. – Peter Lawrey Dec 03 '12 at 22:22
  • A Set would improve performance by ensuring there are no duplicates. If you create a List from a HashSet, the List will also have no duplicates. – jahroy Dec 03 '12 at 22:25
  • @user1068446 - If you're worried about converting between Set and List, check out the second half of my answer. – jahroy Dec 03 '12 at 22:28
4

You can't modify a Collection while you're iterating over it with an Iterator.

Your attempt to call iter.remove() breaks this rule (your removeWord method might, too).

You CAN modify a List while iterating IF you use a ListIterator to iterate.

You can convert your Set to a List and use a List iterator:

List<String> tempList = new ArrayList<String>(dict);
ListIterator li = tempList.listIterator();

Another option is to keep track of the elements you want to remove while iterating.

You could place them in a Set, for example.

You could then call dict.removeAll() after your loop.

Example:

Set<String> removeSet = new HashSet<String>();
for (String s : dict) {
    if (shouldRemove(s)) {
        removeSet.add(s);
    }
}
dict.removeAll(removeSet);
jahroy
  • 22,322
  • 9
  • 59
  • 108
1

When using a for each loop you are not allowed to modify the Collection you are iterating inside the loop. If you need to modify it, use a classic for loop

andreih
  • 423
  • 1
  • 6
  • 19
  • 1
    It's true that a traditional for loop will avoid a _ConcurrentModificationException_. However, you can't access elements in a Set by their index. – jahroy Dec 03 '12 at 22:26
1

This is a common occurance in all Collections classes. For instance the entry in TreeSet uses failfast method.

The iterators returned by this class's iterator method are fail-fast: if the set is modified at any time after the iterator is created, in any way except through the iterator's own remove method, the iterator will throw a ConcurrentModificationException. Thus, in the face of concurrent modification, the iterator fails quickly and cleanly, rather than risking arbitrary, non-deterministic behavior at an undetermined time in the future.

http://docs.oracle.com/javase/6/docs/api/java/util/TreeSet.html

CodeDreamer
  • 444
  • 2
  • 8