CODEWITHSUNDEEP
javaarrays
shanethehat
shanethehat

Reputation: 15570

Why does this code throw an ArrayIndexOutOfBoundsException?

Here's the code:

public class Deck {

    private Card[] cards;

    public Deck() {
        cards = new Card[52];
        String[] ranks = {"ace","two","three","four","five","six","seven","eight","nine","ten","jack","queen","king"};
        String[] suits = {"hearts","diamonds","clubs","spades"};
        for(int i = 0; i < suits.length; i++) {
            for(int n = 0; n < ranks.length; n++) {
                cards[cards.length] = new Card(ranks[i],suits[n]);
            }
        }
    }
}

As you can see, this loops though the two given arrays and generates a card for every combination. There are 13 ranks x 4 suits = 52 cards. I expected that on the 52nd iteration, cards.length would be 51, but the compiler says

Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: 52
    at com.cards.Deck.<init>(Deck.java:14)

Why is that?

Upvotes: 1

Views: 1610

Answers (4)

Brian Roach
Brian Roach

Reputation: 76898

cards[cards.length]

Because you're trying to use an index that doesn't exist. cards.length is 52, your array is 0 - 51.

I suspect you're trying to insert each card into that array, which means you need another counter ;)

int cardIndex = 0;
for(int i = 0; i < suits.length; i++) {
    for(int n = 0; n < ranks.length; n++, cardIndex++) {
        cards[cardIndex] = new Card(ranks[n],suits[i]);
    }
}

EDIT: What I didn't catch is what others have mentioned - you also have the counters for ranks/suits switched in the Card constructor - fixed that too.

Upvotes: 2

templatetypedef
templatetypedef

Reputation: 372784

The issue is that cards.length is not the total number of elements used in the array; it's the total number of elements in the array, regardless of what you've stored in the array so far. Consequently, as soon as you execute the inner loop, this will try accessing the 52nd element of the array, causing the exception you've seen.

To fix this, consider instead storing a counter that will keep track of the next free index, or use some simple math to derive the position that the card should go in from its suit and value. For example, since on each iteration of the outer loop you will write ranks.length elements to the array, on iteration (i, n) you will write to array index i * ranks.length + n. Using this, you could rewrite the inner loop as

    // Careful... still buggy!
    for(int i = 0; i < suits.length; i++) {
        for(int n = 0; n < ranks.length; n++) {
            cards[i * ranks.length + n] = new Card(ranks[i],suits[n]);
        }
    }

Additionally, note that your access into the arrays is wrong. Right now, you're writing

new Card(ranks[i],suits[n]);

However, i ranges over suits, not values. The proper code would be

new Card(ranks[n],suits[i]);

Which gives this final implementation:

    for(int i = 0; i < suits.length; i++) {
        for(int n = 0; n < ranks.length; n++) {
            cards[i * ranks.length + n] = new Card(ranks[n],suits[i]);
        }
    }

More generally, though, don't use the .length field of an array to track how many used elements there are. You'll need to store that separately. Alternatively, consider using an ArrayList, which wraps an array and tracks this for you.

Hope this helps!

Upvotes: 2

user166390
user166390

Reputation:

Consider it with renamed variables:

for(int SUIT = 0; SUIT < suits.length; SUIT++) {
    for(int RANK = 0; RANK < ranks.length; RANK++) {
        cards[cards.length] = new Card(ranks[SUIT],suits[RANK]);
    }
}

It isn't always best to use i and n. (I would have used s and r.)

Also, consider:

Card[] cards = new Card[X];
cards[X] // will never be "in bounds", indices from [0, X-1]

Happy coding.

Upvotes: 0

ddyer
ddyer

Reputation: 1788

The index variables for ranks ranks and suits are exchanged. (Don't damage your head with your palm.)

Upvotes: 0

Related Questions