Simple Blackjack implementation

Continuing my previous post, I have completed my game of Blackjack, the simple version. I am aware it's very long. I haven't included some of the suggested changes suggested in the previously to the Deck implementation and the other classes involved.

It is simple because I omitted some rules (I am planning on implementing them in the future):

• Double

• Split (I think this would be the hardest one)

• Retire

• Dealer checking second card when first is Ace or figure

About the Player and Dealer classes:

I am almost tempted to make a superclass since they share too many class attributes and methods. But I think the idea of "dealer" and "player" are very different concepts even though in this situation they share many features. Furthermore, I have no clue how I would name the superclass.

I would love suggestions on how to make this game better and also approaches about the missing features.

Code on Pastebin.

```
import java.util.Stack;
import java.util.Random;
import java.util.Collections;
import java.util.List;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Scanner;
import java.util.Iterator;

public class Blackjack {

public static String capitalizeFirstLetter(String str) {
return str.substring(0, 1).toUpperCase() + str.substring(1).toLowerCase();
}

public static class Rank {

private int value;
private static final String[] string_values = {
"Ace",
"Two",
"Three",
"Four",
"Five",
"Six",
"Seven",
"Eight",
"Nine",
"Ten",
"Jack",
"Queen",
"King"
};

public Rank(int value) throws IllegalArgumentException {
if (value 13) {
throw new IllegalArgumentException("Invalid card value (must be between 1 and 13).");
}
this.value = value;
}

public Rank(String value) thro

You don't mention why you didn't follow the advice from your previous post about using enum values rather than custom classes. Note that you can customize an enum.

Overall, I think you make too much use of interior classes. There are times to do that, but this doesn't seem to be one of them. Note that Deck, Suit, Value, and Card are all more general concepts that exist outside of blackjack. Hand also exists in other games, but the rules are different. Perhaps Hand should stay in Blackjack. It would be more common to put each class in separate files in a common package named something like tld.domain.blackjack.

Player player1, player2, player3;
        player1 = new Player("Player 1", 500);

        Player[] players = { player1 };

You never use player2 or player3. You don't actually need any of them. You could just say

Player[] players = { new Player("Player 1", 500) };

As a general rule, if you are numbering your variables, you are probably doing something wrong.

private static final int number = 52;

I'm guessing that you intend this as the number of cards in a deck. A more intuitive name is something like CARDS_PER_DECK or DECK_SIZE. But of course, you never use this constant, so you can get rid of it.

Suit hearts, diamonds, clubs, spades;

            hearts = new Suit("hearts");
            diamonds = new Suit("diamonds");
            clubs = new Suit("clubs");
            spades = new Suit("spades");

            Suit[] suits = { hearts, diamonds, clubs, spades };

            for (int i = 0; i < suits.length; i++) {
                for (int j = 1; j <= 13; j++) {
                    deck.push(new Card(new Rank(j), suits[i]));
                }
            }

You have a magic number (13) here. You should get rid of it. The obvious solution would be a constant, but we can do better with enums for suits and ranks.

for (Suit suit : Suit.values()) {
                for (Rank rank : Rank.values()) {
                    deck.push(new Card(rank, suit));
                }
            }

This is not only more idiomatic but shorter. If you really wanted, you could do this with arrays instead. But enums fit this situation exactly.

public static class Player {

        private String name;
        private int money;
        private int bet;
        private Hand hand;
        private State state;

The state and bet variables should be a characteristic of a Hand, not a Player. Either there should be a collection of hands per player, or Hand should not be part of Player at all. Doing either of those is going to make it much easier to implement split.

This is also why your Player and Dealer classes share so many methods. You put a bunch of methods on them that belong on Hand instead. I'm not sure that you even need a Dealer class. It might make more sense to track a Table which would have a collection of players or hands (or both).

public void busted() {
            state = State.BUSTED;
        }

This would make more sense if it was called bust rather than busted. But adding a card to a hand should change the hand's state, not a call from outside. I.e. you shouldn't need this because Hand should change the state internally.

char choice;
            do {
                p(name + ", choose your play; (H)IT, (S)TAND, (D)OUBLE, S(P)LIT OR S(U)RRENDER:");
                pnln("> ");
                choice = Character.toLowerCase(in.next().charAt(0));
            } while (!(choice == 'h' || choice == 's' || choice == 'd' || choice == 'p' || choice == 'u' ));

            switch(choice) {
                case 'h': return Play.HIT;
                case 's': return Play.STAND;
                case 'd': return Play.DOUBLE;
                case 'p': return Play.SPLIT;
                case 'u': return Play.SURRENDER;
                default: return null;
            }

If you move the switch into the loop, you can simplify things and make them more extensible.

while (true) {
                System.out.println(name + ", choose your play; (H)IT, (S)TAND, (D)OUBLE, S(P)LIT OR S(U)RRENDER:");
                System.out.print("> ");
                char choice = Character.toLowerCase(in.next().charAt(0));

                switch (choice) {
                    case 'h': return Play.HIT;
                    case 's': return Play.STAND;
                    case 'd': return Play.DOUBLE;
                    case 'p': return Play.SPLIT;
                    case 'u': return Play.SURRENDER;
                }
            }

This way adding or removing an option only requires a change in one place.

I find it confusing that p calls System.out.println while pnln calls System.out.print.

StackExchange Code Review Q#97605, answer score: 3