Why does this code throw an ArrayIndexOutOfBoundsException?

Question

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?

templatetypedef · Accepted Answer · 2011-09-15 00:10:11Z

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!

Brian Roach · Accepted Answer · 2011-09-15 00:12:51Z

2

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.

edited Sep 15, 2011 at 0:12

answered Sep 15, 2011 at 0:04

Brian Roach

77k12 gold badges140 silver badges164 bronze badges

Comments

ddyer · Accepted Answer · 2011-09-15 00:07:39Z

0

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

answered Sep 15, 2011 at 0:07

ddyer

1,80419 silver badges27 bronze badges

2 Comments

shanethehat Over a year ago

That wasn't the issue, but it would have been the next one!

Brian Roach Over a year ago

That still doesn't fix his problem ... ;)

user166390 · Accepted Answer · 2011-09-15 00:07:42Z

0

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.

answered Sep 15, 2011 at 0:07

user166390

2 Comments

shanethehat Over a year ago

That wasn't the issue, but it would have been the next one!

ddyer Over a year ago

While we're talking better coding practices: use and new Card[suits.length*cards.length] ... for(String S:suits) ... for(String C:cards) ...

Collectives™ on Stack Overflow

Why does this code throw an ArrayIndexOutOfBoundsException?

4 Answers 4

Comments

Comments

2 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

2 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related