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.

Answer 1

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];

Answer 2

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.

Answer 3

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