Is the way I'm handling deck (card) generation the wisest way to do it?

My Deck Class:

(In case it isn't clear, a Card is just an object with two enums, one for suit and one for value).

I'm particularly proud of the readability of this little bit if I do say so myself.

public class Deck {

    private static Random random = new Random();

    private List cards = new ArrayList();

    Deck () {
        // Create one of each card to populate deck with.
        for (Card.Suit suit : Card.Suit.values()) {
            for (Card.Value value : Card.Value.values()) {
                Card card = new Card(value, suit);
                cards.add(card);
            }
        }
        // Shuffle multiple times to ensure randomness...
        this.shuffle();
        this.shuffle();
        this.shuffle();
    }

    public void shuffle() {
        List temp = new ArrayList();
        while(!cards.isEmpty()) {
            int randInt = random.nextInt(cards.size());
            temp.add(cards.remove(randInt));
        }
        cards = temp;
    }

    public void addCard(Card card) {
        cards.add(card);
    }

    public Card getNextCard() {
        return cards.remove(0);
    }

}

EDIT:

I revised the drawCard() method. What do you guys think?

public Card drawCard() {
    if(cards.isEmpty()) {
        System.err.println("Can't draw from an empty deck!");
        System.exit(-1);
    }
    return cards.remove(cards.size() - 1);
}

I would agree that your implementation is quite readable. Indentation and white space are consistent. The variables and operations are named clearly and concisely, with the exception of 'getNextCard' as noted by '200_success'. This allows us to focus much more at the low level nuts and bolts of your code.

private static Random random = new Random();

Use caution in assigning an instance of the Random class as a static member as concurrent access to its operations can cause contention and consequent poor performance. A pattern I like to follow to ensure random instances are seeded uniquely is to have a package level static random member for seeding instances. This still risks contention, but limits the surface area for it to occur in. For simple gaming purposes the 48-bit seed of Random will suffice, but using java.security.SecureRandom will provide a better source of entropy:

private static final SecureRandom SEEDER = new SecureRandom();
    private static final int ENTROPY = 256; // 2048-bit is good enough for banking
    private SecureRandom random = new SecureRandom(SEEDER.generateSeed(ENTROPY));

-

this.shuffle();
    this.shuffle();
    this.shuffle();

If your shuffle method and random source are fair, than one shuffle is actually better than more -- you're skipping over entropy and risk introducing bias. If either is not fair shuffling more will not make them fair.

public void shuffle() {
    List temp = new ArrayList();
    while(!cards.isEmpty()) {
        int randInt = random.nextInt(cards.size());
        temp.add(cards.remove(randInt));
    }
    cards = temp;
}

While I would argue with mjolka that your implementation of shuffle is a non-in-place implementation of the Fisher-Yates shuffle, I would prefer to be more concise and use the base library utility method:

public void shuffle() {
    java.util.Collections.shuffle(cards, random);
}

I would join both mjolka and 200_success in recommending to guard your getNextCard implementation and throw an appropriate error as well as exposing an external status check method such as size() or isEmpty(). Also I agree the card should be popped from the end of the list when using an ArrayList as the backing collection.

Finally, you have a very well formed class with clear and concise public members. Be sure to add just enough javadoc comments to allow a the class to be consumed without needing to open the code.

StackExchange Code Review Q#79838, answer score: 5