Trying to move elements of a list form one to another, until one is empty; ConcurrentModificationException

Question

NB: Please see my edit below:

I've been trying to write a card shuffling simulation java program for a while now. The high level overview is this:

Make a deck
Cut myDeck into my handLeft and handRight
While my handRight is not empty:
- Move a random number of cards from the right hand to the left.

This moving part is done by the method below:

public ArrayList<Card> leftShuffle(int fudge){
    Random randomGenerator = new Random();
    int numberOfCardsToMove = 0;
    while(!handRight.isEmpty()){
        numberOfCardsToMove = randomGenerator.nextInt(fudge);
        if (numberOfCardsToMove<handRight.size()){
            List<Card> cardsBeingMoved = handRight.subList(0, numberOfCardsToMove);
            handRight.removeAll(cardsBeingMoved);  <---line 81
            handLeft.addAll(cardsBeingMoved);      <---line 82
        }else{
            handLeft.addAll(handRight);
            handRight.clear();
        }
    }
    return handLeft;
}

However, when I try to add cards to the SubList cardsBeingMoved java freaks out and gives me this exception:

Exception in thread "main" java.util.ConcurrentModificationException
at java.util.ArrayList$SubList.checkForComodification(ArrayList.java:1091)
at java.util.ArrayList$SubList.size(ArrayList.java:921)
at java.util.AbstractCollection.toArray(AbstractCollection.java:136)
at java.util.ArrayList.addAll(ArrayList.java:497)
at org.Craig.CardShuffling.Scientist.leftShuffle(Scientist.java:82)
at org.Craig.CardShuffling.Scientist.shuffle(Scientist.java:49)
at org.Craig.CardShuffling.Launcher.main(Launcher.java:11)

Now I'm familiar with iterators and how they work, but I don't think the standard while(it.hasNext) loop will work here. I've seen one attempt at a solution here but I'm not sure if it maps neatly on to my problem. Removing something is not causing this issue, adding something unrelated to the while clause is causing the issue. Otherwise I could change the condition to something like while(handLeft.size!=52) and then remove from handRight at the end.

I've tried using the CopyOnWriteArrayList class, but that errors the same way (see appendix A), I've tired various 'holding' lists so that the same list isn't being altered at the same time but it still give the exception.

How do I get past this issue? Is there a clever way to use iterators I've not thought of yet?

Appendix A

Shuffle Method

    public ArrayList<Card> shuffle (int fudge, Shuffling method){
    cutDeck(fudge);
    switch(method){
    case right:
        myDeck.setDeck(rightShuffle(fudge));
        break;
    case left:
        myDeck.setDeck(leftShuffle(fudge));
        break;
    case both:
        myDeck.setDeck(bothShuffle(fudge));
        break;
    }
    return myDeck.getDeck();
}

CopyOnWriteArrayList exception stack trace.

Exception in thread "main" java.util.ConcurrentModificationException
at java.util.concurrent.CopyOnWriteArrayList$COWSubList.checkForComodification(CopyOnWriteArrayList.java:1130)
at java.util.concurrent.CopyOnWriteArrayList$COWSubList.size(CopyOnWriteArrayList.java:1170)
at java.util.AbstractCollection.toArray(AbstractCollection.java:136)
at java.util.concurrent.CopyOnWriteArrayList.addAll(CopyOnWriteArrayList.java:770)
at org.Craig.CardShuffling.Scientist.leftShuffle(Scientist.java:82)
at org.Craig.CardShuffling.Scientist.shuffle(Scientist.java:49)
at org.Craig.CardShuffling.Launcher.main(Launcher.java:11)

Appendix B:

I've changes the method to this:

    public ArrayList<Card> leftShuffle(int fudge){
    Random randomGenerator = new Random();
    int numberOfCardsToMove = 0;
    while(!handRight.isEmpty()){
        numberOfCardsToMove = randomGenerator.nextInt(fudge);
        if (numberOfCardsToMove<handRight.size()){
            List<Card> cardsBeingMoved = handRight.subList(0, numberOfCardsToMove);
            handRight.removeAll(cardsBeingMoved);
            /* cardBeingMoved is just a view, 
             * clearing it removes the same cards from handRight*/
            cardsBeingMoved.clear();
        }else{
            handLeft.addAll(handRight);
            handRight.clear();
        }
        System.out.println("Size of Deck one:" + handRight.size() + "\n");
        System.out.println("Size of Deck one:" + handLeft.size() + "\n");
    }
    return handLeft;
}

Now the exception is this.

Exception in thread "main" java.util.ConcurrentModificationException
at java.util.ArrayList$SubList.checkForComodification(ArrayList.java:1091)
at java.util.ArrayList$SubList.size(ArrayList.java:921)
at java.util.AbstractList.clear(AbstractList.java:234)
at org.Craig.CardShuffling.Scientist.leftShuffle(Scientist.java:86)
at org.Craig.CardShuffling.Scientist.shuffle(Scientist.java:49)
at org.Craig.CardShuffling.Launcher.main(Launcher.java:11)

Please post Scientist.shuffle() method – Aubin Feb 16 '13 at 21:49 — Aubin, Feb 16 '13 at 21:49

score 2 · Accepted Answer · answered Feb 16 '13 at 21:49

2

A sublist is a view, not a copy. So you're removing the elements of cardsBeingMoved at the same time you're trying to use it for something else.

Instead, do the following:

 handLeft.addAll(cardsBeingMoved);
 cardsBeingMoved.clear();

answered Feb 16 '13 at 21:49

Louis Wasserman

191,574
25
345
413

I had no idea it was just a view! If I clear the view, will it only remove the elements from `cardsBeingMoved`? Going to try this out now. – AncientSwordRage Feb 16 '13 at 21:59
2

No, it'll remove the cards from `handRight` as well. In fact, the documentation for `Lists.subList` [specifically suggests](http://docs.oracle.com/javase/6/docs/api/java/util/List.html#subList(int,%20int)) `List.subList(5, 10).clear()` as an idiom for removing elements in that range of the list. – Louis Wasserman Feb 16 '13 at 22:10
Louis, I meant to say "If I clear the view, will it only remove the elements *in `handRight`* from `cardsBeingMoved`?" Regardless, it doesn't work. See my appendix B. – AncientSwordRage Feb 16 '13 at 22:15
PICNIC: I fixed it so the code reflects this now. And it works. Thanks very much. – AncientSwordRage Feb 17 '13 at 12:57

score 1 · Answer 2 · answered Feb 16 '13 at 21:49

the subList function creates a list which is backed by the initial list i.e. all operations are reflected from sublist -> list and vice versa. So the code below is not valid:

List<Card> cardsBeingMoved = handRight.subList(0, numberOfCardsToMove);
handRight.removeAll(cardsBeingMoved);  <---line 81
handLeft.addAll(cardsBeingMoved);      <---line 82

score 1 · Answer 3 · answered Feb 16 '13 at 22:48

Instead of using subList, how about using loop to move the cards from handRight to handLeft one by one, like this:

    public ArrayList<Card> leftShuffle(int fudge){
    Random randomGenerator = new Random();
    int numberOfCardsToMove = 0;
    while(!handRight.isEmpty()){
        numberOfCardsToMove = randomGenerator.nextInt(fudge);
        if (numberOfCardsToMove<handRight.size()){
            for(int i = 0; i < numberOfCardsToMove; i++){
                handLeft.add(handRight.get(i));
                handRight.remove(i);
            }
        }else{
            handLeft.addAll(handRight);
            handRight.clear();
        }
    }
    return handLeft;
}

Trying to move elements of a list form one to another, until one is empty; ConcurrentModificationException

Appendix A

Appendix B:

3 Answers3