Basic Poker Draw

I humbly ask that you review my simple (albeit a little over-done) program. The intention is to create a deck of cards, shuffle them, and draw the first five off the deck as a poker hand.

Can you offer some feedback, particularly if something can be done more efficiently? I intentionally left out much of the error-correction that can go into this just to cement the object-based design.

CardEnum.java

public enum CardEnum {
    ACE("A"), TWO("2"), THREE("3"), FOUR("4"),
    FIVE("5"), SIX("6"), SEVEN("7"), EIGHT("8"),
    NINE("9"), TEN("10"), JACK("J"), QUEEN("Q"), KING("K");

    private String name;

    CardEnum (String name) {
        this.name = name;
    }

    String getSymbol() {
        return this.name;
    }
}

SuitEnum.java

public enum SuitEnum {
    HEART("H"), DIAMOND("D"), CLUB("C"), SPADE("S");

    private String name;

    SuitEnum (String name) {
        this.name = name;
    }

    String getSymbol() {
        return this.name;
    }
}

Card.java

public class Card {
    private CardEnum card;
    private SuitEnum suit;

    private Card() {}

    public Card(CardEnum card, SuitEnum suit) {
        this.card = card;
        this.suit = suit;
    }

    void changeCard(CardEnum card, SuitEnum suit) {
        this.card = card;
        this.suit = suit;
    }

    String getName() {
        return card.getSymbol() + suit.getSymbol();
    }

    CardEnum getCard() { return card; }

    SuitEnum getSuit() { return suit; }
}

Deck.java

```
import java.util.*;

public class Deck {
private final int newSize = 52;
private List deck = new ArrayList<>(newSize);

public Deck() {
for (CardEnum c : CardEnum.values()) {
for (SuitEnum s: SuitEnum.values()) {
Card newCard = new Card(c, s);
this.deck.add(newCard);
}
}

if (deck.size() != newSize)
throw new IndexOutOfBoundsException("Deck does not contain 52 cards.");
}

int s

• Using name coupled with getSymbol is confusing - the underlying property name should be the same.

• Do not omit public/private/default modifiers.

-
Get rid of this method:

// A card should be immutable.
// This method exposes the guts for no good reason.
// Gives you a chance to create an invalid deck.
void changeCard(CardEnum card, SuitEnum suit) {
    this.card = card;
    this.suit = suit;
}

-
Depending on how frequently the cards are reused and accessed, you might want to compute this value only once, and then keep returning it.

// This could be computed only once.
String getName() {
    return card.getSymbol() + suit.getSymbol();
}

-
I like more verbose variable names and some things ought to take one line whereas others ought not. I would write:

public Deck() {
    for (CardEnum card : CardEnum.values()) {
        for (SuitEnum suit: SuitEnum.values()) {
            this.deck.add(new Card(card, suit)); // I find this readable.
        }
    }

    if (deck.size() != newSize) {
        throw new IndexOutOfBoundsException("Deck does not contain 52 cards.");
    } 
    // If statements without braces can be dangerous when you change the code later.
    // I personally do not like them here or elsewhere.
}

-
You might want to make the deck reusable by not throwing cards away but storing them in a dirty pile(say an array). Because a deck will always contain 52 cards, I would use two arrays of 52 elements each - one for clean cards and one for the dirty ones. Then you can make these arrays behave like queues when you need them to by keeping track of exactly one number: the number of cards remaining in the clean pile. You can fetch the cards from 51st to 0th. That way you do not even need to erase the contents of the clean pile - by decreasing the number you hide one more card from the clean deck. This should make your program a bit smaller and faster, but maybe this is premature optimization.

-
You could add an bool isEmpty() and bool isFull() method to the Deck class.

-
You could add automatic re-shuffling by passing a bool flag to the constructor. It should default to false via constructor chaining. This can be useful in the case when you have many players and the game requires multiple decks, but this does not apply to poker. I am not sure what kind of system you are trying to build.

-
You only have about two pages of Java code. You need to expand on this. It looks hygienic precisely because it is not doing all that much.

Looks good otherwise. Keep at it.

StackExchange Code Review Q#10583, answer score: 3