ArrayIndexOutOfBoundsException error in java

[Rocky Rocha]

I made a Deck class in java. the point of the Deck class is to manage a set of Cards. In the constructor of the Deck class, it generates a list of Cards that make up the Deck. I am getting an error that is as follows:

Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: 4
	at Deck.<init>(Deck.java:12)
	at Main.main(Main.java:7)

The code for the Deck class is

import java.util.ArrayList;
import java.util.Random;

public class Deck {

	private ArrayList<Card> cards = new ArrayList<Card>();
	private int size;
	
	public Deck(String[] ranks, String[] suits) {
		for(int a = 0;a <= ranks.length-1;a++)
			for(int b = 0;b <= suits.length-1;b++)
				cards.add(new Card(suits[a],ranks[b]));
		size = cards.size();
		shuffle();
	}
	
	/**
	 * returns true if the deck is empty
	 * returns false otherwise
	 * @return whether or not the deck is empty
	 */
	public boolean isEmpty() {
		if (size == 0)
			return true;
		else
			return false;
	}
	
	/**
	 * returns the size of the deck
	 * @return the size of the deck
	 */
	public int size() {
		return size;
	}
	
	/**
	 * deals a card
	 * @return the card at the top of the deck
	 */
	public Card deal() {
		if (isEmpty())
			return null;
		else
			return cards.get(--size);
	}
	
	/**
	 * shuffles the deck n times
	 */
	public void shuffle(int numberOfTimes) {
		for(int x = 0;x < numberOfTimes;x++) {
			int[] valuesUsed = new int[size];
			Random rnd = new Random();
			for(int i = 0;i < size;i++) {
				Card tmp = cards.get(size-1);
				int rand = rnd.nextInt(size-1);
				for (int y = 0;y < i;y++) {
					if(valuesUsed[y] == rand)
						rand = rnd.nextInt(size-1);
				}
				cards.set(size, cards.get(rand));
				cards.set(rand, tmp);
			}
		}
	}
	
	/**
	 * shuffles the deck
	 */
	public void shuffle() {
		int[] valuesUsed = new int[size];
		Random rnd = new Random();
		for(int i = 0;i < size;i++) {
			Card tmp = cards.get(size);
			int rand = rnd.nextInt(size-1);
			for (int y = 0;y < i;y++) {
				if(valuesUsed[y] == rand)
					rand = rnd.nextInt(size-1);
			}
			cards.set(size, cards.get(rand));
			cards.set(rand, tmp);
		}
	}
}

and the code for the main class is

public class Main {

	static String[] suits = {"Hearts","Diamonds","Clubs","Spades"};
	static String[] ranks = {"Ace","2","3","4","5","6","7","8","9","10","Jack","Queen","King"};
	
	public static void main(String[] args) {
		Deck deck = new Deck(suits,ranks);
		deck.shuffle(3);
		
	}

}

Why am I getting this error?

[Tim Holloway]

I think you have suits[a], ranks[b] where you should have written suits[b], ranks[a].

Also, you might find this syntax easier to work with:

for ( int i = 0; i < suits.length; i++) {
   for (int j=0; j < cards.length; j++ ) {
     cards.add( new Card(suits[i], cards[j]);
  }
}
this.size = cards.size();
shuffle();

The use of i, j, and k as indexes is a carryover convention from common mathematics usage, although for all Java cares, you could use "fred' and "bill" (though Fred and Bill would violate the capitalization conventions).

The i = 0; i < something.length syntax plays to the fact that Java uses zero-based indexes on arrays. Note that by using "<" instead of "<=" you don't need to remember to add the "-1".

Also, I'm snippy about lots of curly brackets myself. I've been burned when I inserted extra statements in a loop or conditional that didn't have them. It's not like we're still saving code on 80 KB floppies.

[Rocky Rocha]

Thanks so much! I guess I was just so tired I couldn't realize that is was such a simple error. Also, to respond to your comments about syntax, I still code for efficiency. Unnecessary characters are unnecessary.

[Tim Holloway]

NEVER code for efficiency. You can always make a well-coded program better (if you need to), but a prematurely-optimized program is not only much more difficult to maintain, it's also much more difficult to re-optimize when it turns out that the original design wasn't as optimal as you thought. If you don't believe me, ask anyone in our Optimization forum. Or consult a good book on best practices in software engineering.

In any event, use of spaces, comments, blank lines, brackets, etc. has zero effect on code efficiency. All that stuff is stripped out early in the lexical processing phase of the compiler. The only "efficiency" you'll gain there is whatever keystrokes you save and I doubt there's any place on Earth that will reward you for saving keystrokes.

Well, actually, there probably is, since the insanity and inanity of shop standards knows no bounds, but I sincerely hope I never walk into one of those places.

[Bear Bibeault]

Rocky Rocha wrote:Unnecessary characters are unnecessary.

Except that the "extra" characters for clarity are not unnecessary.

And, what Tim said.

[Tim Holloway]

It's not clarity, it's self-defense:

if ( notwogg >= 947.325 ) 
   // Temporary line added for debugging:
   System.out.println("notwogg has reached the detonation limit. Reset system to prevent nuclear destruction. value=" + notwogg );
   notwogg = 0.0;

Like I said, I've been burned.

[Rob Spoor]

Tim Holloway wrote:I think you have suits[a], ranks[b] where you should have written suits[b], ranks[a].

Also, you might find this syntax easier to work with:

for ( int i = 0; i < suits.length; i++) {
   for (int j=0; j < cards.length; j++ ) {
     cards.add( new Card(suits[i], cards[j]);
  }
}
this.size = cards.size();
shuffle();

Using for-each loops makes it even easier, and there is no risk of getting your indexes mixed up:

for (String suit : suites) {
    for (String card : ranks) {
        cards.add(new Card(suit, rank));
    }
}

[Tim Holloway]

Good point, although I usually go even one step further:

enum Suits {
  Hearts,
  Clubs,
  Spades,
  Diamonds
};

enum Cards {
  Ace,
  Two,
  Three,
  ...
  Jack,
  Queen,
  King
};

Hope I got the syntax right. I spent too many years using C/C++ enums and there are some differences. Using enums makes things typesafe in addition to providing extra flexibility in calculations and formatting.

[Rob Spoor]

Apart from the semi-colons after the enums those are just fine. Although I'd put Ace after King

[Campbell Ritchie]

Rocky Rocha wrote:. . . The code for the Deck class is // . . . public boolean isEmpty() { if (size == 0) return true; else return false; } . . .

Have a look at the old Sun style guide, which tells you that sort of code is poor style. Write this instead:- // . . . public boolean isEmpty() { return (size == 0); }The () around == are redundant: read redundant as meaning optional rather than unnecessary.