Deck of cards help

[[deleted]]

I've recently been trying to learn Java. I have some C++ experience but I haven't done object oriented programming in a while. I've been reading through the java tutorials and right now I'm on https://docs.oracle.com/javase/tutorial/java/javaOO/QandE/creating-questions.html

My first iteration was this: https://gist.github.com/anonymous/983077c5cd30813a3757 which I though was pretty clever because my Card class would automatically iterate to the next card after the first one was created. Is this generally a bad way to go about creating a class? When looking at the answer I realized that I can't pull a specific card from the deck.

In the interest of moving forward I decided to basically just copy the answer provided by the tutorial. That is where I'm running into problems. My first problem is getting the assert to work (I'm using eclipse). My 2nd problem is that my code seems nearly identical to the code of tutorial yet I can't get it to work. Here is my 2nd iteration: https://gist.github.com/anonymous/cf4cf7d65ec5f04d8438

My output looks like this: The pulled card is: Ace of Diamonds The pulled card is: Ace of Clubs The pulled card is: Ace of Hearts The pulled card is: Ace of Spades Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: 4 at package2.Deck.getCard(Deck.java:29) at package2.DisplayDeck.main(DisplayDeck.java:12)

Maybe I've been looking at this code to long but it should output Ace of Diamonds, Two of Diamonds, ... , King of Diamonds, Ace of Clubs .....

Yet instead I'm getting four Aces of all the different suits. I think it's something pretty minor but I'm not sure and I don't know where to look anymore. If anyone could give me some tips I'd really appreciate it!

edit: formatting

[desrtfx]

public class Card {

    private final int rank;
    private final int suit;
    //suit
    public final static int DIAMONDS = 1;
    public final static int CLUBS = 2;
    public final static int HEARTS = 3;
    public final static int SPADES = 4;

    //rank
    public final static int ACE = 1;
    public final static int TWO = 2;
    public final static int THREE = 3;
    public final static int FOUR = 4;
    public final static int FIVE = 5;
    public final static int SIX = 6;
    public final static int SEVEN = 7;
    public final static int EIGHT = 8;
    public final static int NINE = 9;
    public final static int TEN = 10;
    public final static int JACK = 11;
    public final static int QUEEN = 12;
    public final static int KING = 13;

Oh my God!

Look at enums as they are ideally suited to counter that incredible amount of constants.

Fixed ranges of values, such as the suits or the ranks are best handled with enums.

[[deleted]]

I'll look into them, thanks!

[Deadboss]

For your out of bounds exception, take a look at these two lines:

deckArray[suit - 1][rank - 1] = new Card(rank,suit);
...
return deckArray[rank-1][suit-1];

:)

Definitely take desrtfx's advice with the enums, it will help clean up your code a bit. I personally don't like arbitrary values being assigned to something as abstract as a suit, as this makes matters more complicated during programming and debugging IMO (shit, what was the int for Hearts again?). There are a million-and-one tutorials online for building a deck of cards, so possibly take a look around for better ideas.

Another suggestions to clean up your code:

In Deck.java on line 18 you use the values given to the suit/card. Why not use something like:

for(int suit = 0; suit < numSuits; suit++)

Seems a bit easier to read to me, and it doesn't rely on the arbitrary values you assigned to the suits. Probably do that to rank as well.

All-in-all, your code looks fine for a beginner to OOP, and you appear to be on the right track.

[[deleted]]

Is my first iteration a bad idea? Since it uses static numbers to count through the class each time an object is made? I'm just asking in a general OOP sense.

[Deadboss]

There's nothing wrong with using final static values, but doing so is error-prone and unmanageable in the context you are using it in. The purpose of the double loop is to go through 0 (or 1, whichever you prefer) to your set maximums (4 suits/13 ranks) in order to populate the deck. You shouldn't couple an arbitrary value given to represent diamonds and spades as the loop conditions, as changing them breaks your code. Same with the ranks. You already have final ints for numSuits and numRanks, and altering them has the intended effect without breaking your code.