UNO card game remove card from hand

Question

Trying to create the card game "UNO" in java. When a player plays a card, it should be removed from the hand with the other elements shifting to the left. It takes an int n as the parameter, which refers to the card being discarded. The method should change the cards array that I have specified as a field of the class. It's an array of objects which are the cards, or the players hand. When ran, it produces a nullPointerException. I know why the error is occurring, im just not sure how to fix it. I'm also trying to avoid the use of Array Lists. It also returns the card that is being discarded so it can be printed. Thanks.

public Card removeCardFromHand(int n)
{
    Card c = cards[n];
    Card[] tempCards = new Card[cards.length - 1];
    for(int i = 0; i < n; i++)
    {
        tempCards[i] = cards[i];
    }
    for(int i = n; i < cards.length; i--)
    {
        tempCards[n] = cards[n + 1];
    }
    cards = tempCards;
    return c;
}

Error Code:

java.lang.ArrayIndexOutOfBoundsException: 7

at Player.removeCardFromHand(Player.java:86)
at BUno.executeOnePlay(BUno.java:112)
at BUno.play(BUno.java:70)
at BUno.main(BUno.java:186)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
at java.lang.reflect.Method.invoke(Unknown Source)
at edu.rice.cs.drjava.model.compiler.JavacCompiler.runCommand(JavacCompiler.java:271)

It's occurring because, in this case, the player had 7 cards. When the 7th one was removed, that 7th index was then empty. I wrote a similar method for adding a card when a player has to draw a card, which worked flawlessly. I am practicing for an upcoming exam, which doesn't cover array lists or vectors, so it's useless for me to use them.

Why don't you want to use an `ArrayList`? *"I know why the error is occuring"* Do share. Also, please paste the entire stack trace of the exception. — Jeffrey, Apr 30 '12 at 02:26
The main reason why 'ArrayList' exists is to handle these kind of problems. But you might have a reason for not using it! Besides, I think your in your second for loop, it should be 'i++' (from n to cards.length). — user916315, Apr 30 '12 at 02:36
@stark See [this](http://stackoverflow.com/q/1386275/758280). — Jeffrey, Apr 30 '12 at 02:37

score 2 · Accepted Answer · answered Apr 30 '12 at 02:37

2

for(int i = n; i < cards.length; i--)
{
    tempCards[n] = cards[n + 1];
}

What is that? :-)

Three immediate problems. The first is that you're using n in the array indexes within the loop rather than the correct i.

The second is that, even when you fix that, you're going to go beyond the end of the array.

The third is that you should be incrementing i rather than decrementing it. Decrementing it means that the loop will run forever since i will always be less than cards.length. And, by forever, I mean right up to the point where you start trying to do something with cards[-1] :-)

Instead, you should try:

for (int i = n; i < cards.length - 1; i++)
    tempCards[i] = cards[i + 1];

answered Apr 30 '12 at 02:37

paxdiablo

854,327
234
1,573
1,953

1

Don't you think it should be **i++**? (n – user916315 Apr 30 '12 at 02:39
this loop is to add the rest of the cards that won't be removed into the shorter tempCards array. I'll give your method a shot though – Alex Moss Apr 30 '12 at 02:41
@AlexMoss, sorry, that was a rhetorical question. I knew exactly what it was. My comment of _"What is that?"_ should be read with an expression of incredulity as in _"What on Earth were you thinking when you wrote that?"._ :-) – paxdiablo Apr 30 '12 at 02:43
No probs, @AlexMoss - you may want to look into the possibility of using a data structure that lets you change the deck _in-place_ rather than having to create a brand new array. But that's probably best for another question. – paxdiablo Apr 30 '12 at 02:46

score 1 · Answer 2 · answered Apr 30 '12 at 02:37

Your second for loop is not doing what you expect it to. You are just reassigning tempCards[n] = cards[n+1] over and over again while you decrement i continuously.

for(int i = n; i < cards.length; i--) 
    { 
        tempCards[n] = cards[n + 1]; 
    }

It looks like for an example i starts with a value something like 3, which would be less than cards.length then you decrement i to 2, 1, 0, -1, -2, and so on.

score 0 · Answer 3 · answered Apr 30 '12 at 02:46

You could use a LinkedList instead of relying on an array. Removing and adding an item (a Card) to your player's hand will be faster.

I see two problems with the code you posted. The first one is a potential concurrent access (but you might handle it at a higher level): is it possible, on the same hand, to add a Card when the removal of one Card isn't completely done ?

The second one is here:

for(int i = n; i < cards.length; i--) 
{ 
   tempCards[n] = cards[n + 1]; 
}

You should do a copy in the other direction (i++) otherwise your tempCard will contain something like {Card1, Card2, ..., Card n-1, Card n+1, Card n, Card n -1 ... Card 2, Card 1} or crash with an ArrayIndexOutOfBound if you try to remove a card with an index > card.lengh / 2

UNO card game remove card from hand

3 Answers3