CODEWITHSUNDEEP
javaarraylistiterator
ivanthemeh
ivanthemeh

Reputation: 53

Using next() to call next iteration of ArrayList

Having a little bit of trouble with the iterator next(). Can't seem to get it to work properly. I've been working on this code for a while so I was thinking another set of eyes would help.

This is my deck class with creates a list of Card objects, I'm trying to make a method to grab the next Card in the list starting with the first one:

package blackjack;

import blackjack.Card.Rank;
import blackjack.Card.Suit;
import java.util.*;

public class Deck {

public ArrayList<Card> cards = new ArrayList<>();
int i;
Card next;

public Deck() {
    initializeDeck();

}

public void printDeck() {
    for (Card c: cards)
        System.out.println(c);
}

private void initializeDeck() {
    for (Suit suit : Suit.values()) {
        for (Rank rank : Rank.values()) {
            cards.add(new Card(rank, suit));
        }
    }
}

public Card getNextCard() {
    if (cards.listIterator().hasNext() != true) {
        getNextCard();
    }
    else {
        next = cards.listIterator().next();
    }
      return next; 
}
}

This is my main class where I call the getNextCard() and what I'm thinking it should do is print the first and then the next Card in the list but what it's doing is printing the first Card twice.

package blackjack;

import java.util.*;

public class BlackJack {

public static void main(String[] args) {
    Deck deck = new Deck();
    System.out.println(deck.getNextCard());
    System.out.println(deck.getNextCard());
    }

}

Thanks in advance for any help!

Upvotes: 2

Views: 19768

Answers (3)

Laf
Laf

Reputation: 8205

You always get the first card because you are creating a new iterator each time you call the getNextCard () method. The line

next = cards.listIterator().next();

will always create a new iterator. What you want to do would look like (assuming you want to create a new ListIterator instance each time you have reached the last card of the deck):

private ListIterator<Card> myIterator = null;

public Card getNextCard() {
    if (myIterator == null || !myIterator.hasNext ()) {
        myIterator = cards.listIterator ();
    }

    return myIterator.next(); 
}

Upvotes: 0

mdierker
mdierker

Reputation: 522

In your getNextCard() method, you're creating an iterator every time it's called. Iterators always start at index 0 (although there is a listIterator(index) method), but you shouldn't need it.

Option 1: Keep track of the iterator, and use the same iterator each time. However, this has an important shortcoming not pointed out by anyone else yet. From the Javadoc:

The iterators returned by this class's iterator and listIterator methods are fail-fast: if the list is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove or add methods, the iterator will throw a ConcurrentModificationException.

Translation: If you modify the list in any way outside of the iterator (say, by adding a card to the end of the list), then your iterator breaks. This leads us to option 2:

Option 2: Keep a counter of the index you returned last, and simply return that each time. Something like:

public class Deck {

public ArrayList<Card> cards = new ArrayList<>();
Card next;
int currentCardIndex = -1;

/* The initialization stuff you have above */

public Card getNextCard() {

    currentCardIndex++;

    // If we're at the end, go back to the beginning
    if (currentCardIndex >= cards.size()) {
        currentCardIndex = 0;
    }

    return (next = cards.get(currentCardIndex));
}

And finally Option 3: (not advised): If you really wanted to, you could catch the ConcurrentModificationException and generate a new iterator at that point, but there's not really a reason to unless you need some iterator-specific feature. (The get() call is just as fast as an interator - both are constant time).

Upvotes: 5

Duncan Jones
Duncan Jones

Reputation: 69339

You need to save the iterator returned by cards.listIterator().

Your code creates a new one each time, meaning you always get the first element.

Upvotes: 3

Related Questions