Can't remove object from ArrayList

Question

I'm working on a card game. I can't figure out how to remove a Card from an ArrayList no matter what. This is the code I'm using:

private List<Card> cardDeck = new ArrayList<Card>();

public void removeCard(Card card) {
    for (Iterator<Card> it = cardDeck.iterator(); it.hasNext();) {
        Card nextCard = it.next();
        if (nextCard.equals(card)) {
            cardDeck.remove(card);
            System.out.println("removed " + card);
        }
    }
}

And here's the card class, incase you need it:

    public class Card {

    public Card(Rank rank, Suit suit) {
        this.rank = rank;
        this.suit = suit;
    }

    public Rank getRank() {
        return rank;
    }

    public Suit getSuit() {
        return suit;
    }

    @Override
    public String toString() {
        return getRank().toString().toLowerCase() + " of "
                + getSuit().toString().toLowerCase();
    }

    private Rank rank;

    private Suit suit;

}

I've tried everything but it just won't remove. Any tips?

Answer 1

When you're iterating over a collection, the only way you're meant to remove an item is to call remove on the iterator. So you should use:

if (nextCard.equals(card)) {
    it.remove();
    System.out.println("removed " + card);
}

Note that as you haven't overridden equals, this is really just a reference comparison, so you'll only go into the body of the if statement if nextCard and card are references to the exact same object.

Of course if you just want the method to remove the card, you should be able to just change it to:

public void removeCard(Card card) {
    cardDeck.remove(card);
}

... with the same caveat around equality, of course.

To override equals (and hashCode for consistency) I would first make Card a final class, then write:

public final class Card {
    ...

    @Override
    public boolean equals(Object other) {
        if (!(other instanceof Card)) {
            return false;
        }
        Card otherCard = (Card) other;
        return this.rank == otherCard.rank &&
               this.suit == otherCard.suit;
    }

    @Override
    public int hashCode() {
        int hash = 17;
        hash = hash * 31 + rank.hashCode();
        hash = hash * 31 + suit.hashCode();
        return hash;
    }
}

That's assuming that Rank and Suit are enums (for the reference equality check in equals to be appropriate). You probably want to add nullity checks in the Card constructor, too.