Second attempt at a Blackjack game

I posted yesterday about my first attempt at a Blackjack game. After I received much helpful advice I tried my best to fix up my code. I was wondering what you would think about round two!

Edit: I also wanted to implement the ability to split a hand if you have two of the same card. My User class already has a split method that adds another hand; what do you think would be the best way to go about adding this feature?

Card

package Blackjack;

class Card {
private final int rank;
private final int suit;
private static String[] ranks = { "Joker", "Ace", "Two", "Three", "Four",
        "Five", "Six", "Seven", "Eight", "Nine", "Ten", "Jack", "Queen",
        "King" };
private static String[] suits = { "Clubs", "Diamonds", "Hearts", "Spades" };

Card(int suit, int values) {
    this.rank = values;
    this.suit = suit;
}

public String toString() {
    return ranks[rank] + " of " + suits[suit];
}

public int getRank() {
    return rank;
}

public int getSuit() {
    return suit;
}

public int getValue() {
    int value=0;
    if (rank > 10) {
        value = 10;
    } else if (rank == 1) {
        value = 11;
    } else {
        value = rank;
    }
    return value;
}
}

Deck

package Blackjack;

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

class Deck {
private ArrayList deck;

Deck() {
    deck = new ArrayList();
    for (int i = 0; i < 4; i++) {
        for (int j = 1; j <= 13; j++) {
            deck.add(new Card(i, j));
        }
    }
}

public void shuffle() {
    Random random = new Random();
    Card temp;
    for (int i = 0; i < 200; i++) {
        int index1 = random.nextInt(deck.size() - 1);
        int index2 = random.nextInt(deck.size() - 1);
        temp = deck.get(index2);
        deck.set(index2, deck.get(index1));
        deck.set(index1, temp);
    }
}

public Card drawCard() {
    return deck.remove(0);
}
}

Hand

```
package Blackjack;

import java.util.ArrayList;
import java.util.Arrays;

class Hand {
private ArrayList ha

Structure

Your structure is a lot better. There are a couple of things I would do differently, but generally it's good.

• a Hand doesn't really need a deck, just a card (it makes more logical sense, I think).

• I think you have many Hands in Player because of splitting? That makes sense. But your hasBlackJack etc methods only handle the first hand, so that makes it useless. I would probably move those check methods directly into Hand.

• I would let choiceisHit return a Action enum, which could be Hit, Stay, Split, etc. This will make it a lot easier to add other actions later on (double, split, surrender, etc).

• your main method is too long, it's really hard to find the pieces you are interested in, which will make it really hard to change it or extend it. I would at least create a method for one round of blackjack and one method for the setup at the beginning.

Arrays and Lists

I noted this last time, but as you still convert lists to arrays, I think maybe you don't know how to handle lists on their own?

Generally - if you don't have a good reason to do it differently-, you should stick with the collection you have. If it's an array, handle the array. If it's a list, handle the list.

Eg this:

for (int i = 0; i  0 && handValue > 21) {
        handValue -= 10;
        aceCounter--;
    }
}

Can be rewritten without the array like this:

for (int i = 0; i  0 && handValue > 21) {
        handValue -= 10;
        aceCounter--;
    }
}

Personally, I would handle the value counting in getHandValue on the fly, to avoid the trouble with the aces and the duplication in Hand and Hit (you could also avoid this by just calling Hit twice in your constructor).

You do the same conversion from list to array in a couple of other places as well. Eg this:

public Card getCard(int cardnum) {
    Card[] aHand = new Card[]{};
    aHand = hand.toArray(aHand);
    return aHand[cardnum-1];
}

could be this:

public Card getCard(int cardnum) {
    return hand.get(cardnum);
}

Same in toString, hasBlackJack, hasBusted, etc.

Naming

• your naming isn't always consistent. Eg getNewHand returns nothing, while getHand returns a hand. getNewHand should probably be something like dealHand.

• choiceIsYes retrieves user input, while isyesorno does not. I would use the getX naming pattern you used for the other methods here.

• always use camelCase (see eg ishitorstand).

• again, methods should start with a lower-case character, so they are not confused with classes/constructors.

Misc

• in drawCard you do not check if there is still a card left, which will lead to a nullpointer exception when there aren't.

• you still have your shuffle method which can be replaced by Collections.shuffle(deck);.

StackExchange Code Review Q#92703, answer score: 3