4
\$\begingroup\$

I started coding blackjack from scratch and just finished the basics so I would love some insight. I would really appreciate if any of you could look through it and let me know if there's anything I can do to improve my code. Since I'm doing this on my own knowledge it would really help to learn if there are places where I can avoid redundancy etc. so I can get better when I code from scratch in the future. Currently my code automates the first and second drawing phase and allows the user to hit/stay. If the dealers total < 17 then it will automatically hit until he is over 17. I haven't incorporated any betting yet but I plan on doing that next. Any feedback is appreciated, thank you!

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

public class Deck {

    String[] deck = new String[52];
    int deckPosition = 0;
    int cardValue;
    int top = 0;
    int handTotal = 0;
    int dealerTotal = 0;
    ArrayList<String> hand = new ArrayList();
    ArrayList<String> dealerHand = new ArrayList();

    public void createDeck() {
        String[] cards = {"Ace", "Two", "Three", "Four", "Five", "Six", "Seven",
                            "Eight", "Nine", "Ten", "Jack", "Queen", "King"};
        for(int i = 0; i < 13; i++) {
            deck[deckPosition] = cards[i] + " of Spades";
            deckPosition++;
        }
        for(int n = 0; n < 13; n++) {
            deck[deckPosition] = cards[n] + " of Clubs";
            deckPosition++;
        }
        for(int h = 0; h < 13; h++) {
            deck[deckPosition] = cards[h] + " of Hearts";
            deckPosition++;
        }
        for(int f = 0; f < 13; f++) {
            deck[deckPosition] = cards[f] + " of Diamonds";
            if(deckPosition != 52) {
                deckPosition++;
            }
        }
    }

    public void shuffle() {
        Random sfl = new Random();
        for(int i = deck.length-1; i > 0; i--) {
            int index = sfl.nextInt(i + 1);
            String a = deck[index];
            deck[index] = deck[i];
            deck[i] = a;
        }
    }

    public int getCardValue(String card) {
        String topCard = card;
        if(topCard.startsWith("Two")) {
            cardValue = 2;
        }
        if(topCard.startsWith("Three")) {
            cardValue = 3;
        }
        if(topCard.startsWith("Four")) {
            cardValue = 4;
        }
        if(topCard.startsWith("Five")) {
            cardValue = 5;
        }
        if(topCard.startsWith("Six")) {
            cardValue = 6;
        }
        if(topCard.startsWith("Seven")) {
            cardValue = 7;
        }
        if(topCard.startsWith("Eight")) {
            cardValue = 8;
        }
        if(topCard.startsWith("Nine")) {
            cardValue = 9;
        }
        if(topCard.startsWith("Jack") || topCard.startsWith("Queen") || topCard.startsWith("King") || topCard.startsWith("Ten")) {
            cardValue = 10;
        }
        return cardValue;
    }

    public String playerDraw() {
        String topCard = deck[top];
        hand.add(deck[top]);
        Scanner choice = new Scanner(System.in);
        if(topCard.startsWith("Ace")) {
            if(handTotal + 11 < 22) {
                cardValue = 11;
            } else {
                cardValue = 1;
            }
        }
        getCardValue(topCard);
        handTotal += cardValue;
        return deck[top++];
    }

    public int getHandTotal() {
        return handTotal;
    }

    public ArrayList<String> getHand() {
        return hand;
    }

    public String dealerDraw() {
        String topCard = deck[top];
        dealerHand.add(deck[top]);
        Scanner choice = new Scanner(System.in);
        if(topCard.startsWith("Ace")) {
            if(dealerTotal + 11 < 22) {
                cardValue = 11;
            } else {
                cardValue = 1;
            }
        }
        getCardValue(topCard);
        dealerTotal += cardValue;
        return deck[top++];
    }

    public int getDealerTotal() {
        return dealerTotal;
    }

    public ArrayList<String> getDealerHand() {
        return dealerHand;
    }

    public ArrayList<String> dealerFirstHand() {
        ArrayList<String> firstHand = new ArrayList<>(2);
        firstHand.add(dealerHand.get(0));
        firstHand.add("Hidden Card");
        return firstHand;
    }

    public int dealerFirstTotal() {
        String card = dealerHand.get(0);
        return getCardValue(card);
    }

}
import java.util.ArrayList;
import java.util.Scanner;

public class Blackjack {
    public static void main(String[] args) {
        Deck deck = new Deck();
        Scanner choice = new Scanner(System.in);
        deck.createDeck();
        deck.shuffle();

        System.out.println("------------------------------------------------");
        System.out.println("FIRST DRAWING PHASE");
        System.out.println("------------------------------------------------");
        System.out.printf("Dealers draw: " + "%s" + '\n', deck.dealerDraw());
        System.out.printf("Your draw: " + "%s" + '\n', deck.playerDraw());
        System.out.println("------------------------------------------------");
        System.out.println("SECOND DRAWING PHASE");
        System.out.println("------------------------------------------------");
        System.out.println("Dealers draws");
        deck.dealerDraw();
        System.out.printf("Your draw: " + "%s" + '\n', deck.playerDraw() + "\n");
        System.out.println("Dealers hand: " + deck.dealerFirstHand());
        System.out.println("Dealer hand total: " + deck.dealerFirstTotal() + "\n");
        System.out.println("Your hand: " + deck.getHand());
        System.out.println("Your hand total: " + deck.getHandTotal() + "\n");
        String c;
        do {
            System.out.print("Would you like to hit or stay?: ");
            c = choice.next();
            if (c.equalsIgnoreCase("hit")) {
                System.out.printf("Your draw: " + "%s" + '\n', deck.playerDraw());
                System.out.println("Your hand: " + deck.getHand());
                System.out.println("Your hand total: " + deck.getHandTotal() + "\n");
                if(deck.handTotal > 21) {
                    System.out.println("You busted!");
                    return;
                }
            }
        } while (!(c.equalsIgnoreCase("stay")));

        if(c.equalsIgnoreCase("stay")) {
            ArrayList<String> hand = deck.getDealerHand();
              System.out.print("Revealing Dealers hand: ");
              System.out.println(hand);
              System.out.print("Dealer total: ");
              System.out.println(deck.dealerTotal);
              while(deck.dealerTotal <= 16) {
                  System.out.println("Dealer draws: " + deck.dealerDraw()) ;
                  System.out.println("Dealer hand: " + deck.dealerHand);
                  System.out.println("Dealer total: " + deck.dealerTotal);
              }
              if(deck.dealerTotal > 17 && (deck.dealerHand.size() != 2)) {
                  System.out.println("Dealer total: " + deck.dealerTotal);
              }

              if(deck.dealerTotal > 21) {
                  System.out.println("Dealer busted! You win!");
              }
              else if(deck.dealerTotal < 21 && deck.dealerTotal > deck.handTotal) {
                  System.out.println("Dealer wins!");
              }
              else if(deck.handTotal < 21 && deck.dealerTotal < deck.handTotal) {
                  System.out.println("You win!");
              }

            }
    }
}
```
\$\endgroup\$

2 Answers 2

5
\$\begingroup\$

separation of concerns

your Deck has too much responisibilities - the deck is responsible for holding cards in the deck, this is good. But it also

  • mangages the players (even AI turns!)
  • works as parser (getCardValue)

These responsibilities should be taken out! They violate the Single Responsiblity Principle

proper data Types

avoid using primitives (well, String is not pure primitive, but close to) as data types - create proper classes.

class Player

interface Player{  
    int getHandTotal();
    List<Card> getHand();
    void drawCard(Deck deck);
}

that would also reduce the code duplicates (getPlayerTotal() and getDealerTotal() and others), which also violated the DRY principle

AI players turn (dealers turn)

even though it is a very simple AI turn that you provide - you should put thing together that belong together - in one place (Separation of concerns principle)! so it would be obvious to put this AI Player logic into a AI Player class that implements the Player interface.

for the human player you can create a very simple class that does nothing but react on user input.

class Card

having a proper class also reduces complexity, you can skip the parse methods (getCardValue)

public enum CardValue {
    TWO("Two", 2), THREE ("Three", 3) ...;
}

public enum CardColor {
   SPADES, HEARTS, DIAMONDS, CLUBS
}

public class Card {
    final CardValue value;
    final CardColor color;

    int getValue();
    String toString();
}

Note i didn't implement the whole classes i just added some stubs

hardcoded strings / magic numbers

text / literals you should separate the text from your code. either use a text template document or you define String literals (final static String INTRO = "...";)

And provide proper printMethods that puts all the printing that belongs together in a method.

magic numbers and you should avoid magic numbers (16 - and lots of others) - but provide a proper name for that (final int drawMinimum = 16;) for all those people who don't know Blackjack that good.

Even better: put that logic into a proper method:

while (dealer.isBelowDrawLimit()){...}

commands

it's ok to use command strings hit or stay (even they should be defines as String literals) but a class for these commands would clear up the readability. - or even better a proper inputhandler. Maybe thats overkill in your case BUT your programm is very sensitiv and any mispelling would cause a abnormal programm abort.

programm flow

it is not very intuitive to put programm flow control and application logic into one method - that violates the Integration Operation Segregation Principle.

mabye an idea for how it could look like

void play(){ //new BlackJack().play() from main()
    Deck deck = new Deck(); 
    deck.createDeck();
    deck.shuffle();
    Player player = new HumanPlayer();
    Player dealer = new AiPlayer();
    InputParser parser = new InputParser(new Scanner(System.in));

    printIntro();

    do {
        dealCards(player, dealer, deck, parser);
    }while(!parser.stay() || player.hasDrawnTooMuch()) //excuse my bad english

    if(!player.hasDrawnTooMuch()){
        updateDealer(dealer, deck);
    }
   
    printResult(player, dealer);
}

side notes

  • i don't see any test, hopefully you just forgot to add them
  • use a proper output wrapper (maybe a logger) instead of writing to console directly
\$\endgroup\$
2
\$\begingroup\$

Deck has far too many responsibilities. I'm going to suggest that you separate it out into at least four classes: a Deck, a Hand which is a subclass of Deck, Cards, and a Game or Table.

deckPosition and cardValue are function-local state leaking out into class state, and should not exist in their current location.

Do not represent cards as strings; give them a class that can render to a string for your text user interface (TUI).

Consider using Java streams. There are some fancy ways to generate the entire deck of cards, and also to reduce a hand to a score.

Do not implement your own shuffle; there's one built-in.

Your handTotal is effectively a form of caching. Caching is difficult to get right, and in this case the cost of recalculating a score is insignificant; so I would do the latter.

Your Hidden Card is actually the hole. I'm not convinced that you've implemented its logic correctly, but I've left that for now. I'm equally unconvinced that you've dealt (sorry) with all of the edge cases around win conditions, such as naturals. Pretty sure there's another bug - after the predetermined hands, you don't check whether anyone has busted.

Your \n are hard-coded for systems that can render that line-ending, which will not always be the case. Use %n in a formatted string instead. So this:

println("Dealer hand total: " + deck.dealerFirstTotal() + "\n")

will turn into

printf("Dealer hand total: %s%n", deck.dealerFirstTotal())

Suggested

BlackJack.java

package com.stackexchange.blackjack;

import java.util.Scanner;
import static java.lang.System.out;


public class BlackJack {
    private final Table table = new Table();
    private final Scanner choice = new Scanner(System.in);

    public static void main(String[] args) {
        var tui = new BlackJack();
        tui.play();
    }

    public void play() {
        playPredeterminedHands();
        if (!playPlayer())
            return;

        playDealer();
        if (table.dealerHand.isBust()) {
            out.println("Dealer busted! You win!");
        }
        else out.println(
            switch (table.getWinState()) {
                case DealerWins -> "Dealer wins!";
                case PlayerWins -> "You win!";
                default -> "Tie.";
            }
        );
    }

    private void playPredeterminedHands() {
        table.dealerFirstDraw();
        table.playerDraw();
        out.printf(
                "------------------------------------------------"
            + "%nFIRST DRAWING PHASE"
            + "%n------------------------------------------------"
            + "%nDealer's draw: %s, then a hidden card in the hole"
            + "%nYour draw: %s"
            + "%n%n",
            table.dealerHand.topCard(),
            table.playerHand.topCard()
        );

        table.dealerDraw();
        table.playerDraw();
        out.printf(
                "------------------------------------------------"
            + "%nSECOND DRAWING PHASE"
            + "%n------------------------------------------------"
            + "%nDealer's draw: %s"
            + "%nYour draw: %s"
            + "%n%n",
            table.dealerHand.topCard(),
            table.playerHand.topCard()
        );

        out.printf(
                "Dealer's hand: %s"
            + "%nYour hand: %s"
            + "%n%n",
            table.dealerHand,
            table.playerHand
        );
    }

    private boolean playPlayer() {
        while (true) {
            out.print("Would you like to hit or stay?: ");
            switch (choice.next().toLowerCase()) {
                case "hit" -> {
                    if (!hit())
                        return false;
                }
                case "stay" -> {
                    return true;
                }
                default -> {
                    out.println("Invalid input");
                }
            }
        }
    }

    private boolean hit() {
        table.playerDraw();
        out.printf(
                "Your draw: %s"
            + "%nYour hand: %s"
            + "%n",
            table.playerHand.topCard(),
            table.playerHand
        );
        if (table.playerHand.isBust()) {
            out.println("You busted!");
            return false;
        }
        return true;
    }

    private void playDealer() {
        out.printf("Revealing dealer's hand: %s%n", table.dealerHand);

        while (table.dealerHand.isSafe()) {
            table.dealerDraw();
            out.printf(
                    "Dealer's draw: %s"
                + "%nDealer's hand: %s"
                + "%n",
                table.dealerHand.topCard(),
                table.dealerHand
            );
        }
    }
}

WinState.java

package com.stackexchange.blackjack;

public enum WinState {
    DealerWins,
    Tie,
    PlayerWins
}

Hand.java

package com.stackexchange.blackjack;

import java.util.ArrayList;

public class Hand extends Deck {
    public static final int
        SAFETY = 17,
        LIMIT = 21,
        ACE_HIVAL = 11;

    public Hand() {
        super(new ArrayList<>());
    }

    private static int cardValue(int total, Card card) {
        int value;
        if (
            card.value() != CardValue.Ace ||
            total + ACE_HIVAL > LIMIT
        )
            value = card.value().value;
        else value = ACE_HIVAL;
        return value + total;
    }

    private static int add(int a, int b) { return a + b; }

    public int getTotal() {
        return cards.stream()
            .reduce(
                0,         // identity
                Hand::cardValue,  // accumulator
                Hand::add         // combiner (not called)
            );
    }

    @Override
    public String toString() {
        return super.toString()
            + String.format("%nTotal: %d", getTotal());
    }

    public boolean isBust() {
        return getTotal() > LIMIT;
    }

    public boolean isSafe() {
        return getTotal() <= SAFETY;
    }
}

Card.java

package com.stackexchange.blackjack;

public record Card(
    Suit suit,
    CardValue value
) {
    @Override
    public String toString() {
        return String.format("%s of %s", value, suit);
    }
}

Suit.java

package com.stackexchange.blackjack;

public enum Suit {
    Spades,
    Diamonds,
    Clubs,
    Hearts
}

Table.java

package com.stackexchange.blackjack;

public class Table {
    private final Deck deck = Deck.full();
    public final Hand
        playerHand = new Hand(),
        dealerHand = new Hand(),
        dealerHole = new Hand();

    public Table() {
        deck.shuffle();
    }

    public void playerDraw() {
        deck.dealTo(playerHand);
    }

    public void dealerDraw() {
        deck.dealTo(dealerHand);
    }

    public void dealerFirstDraw() {
        deck.dealTo(dealerHand);
        deck.dealTo(dealerHole);
    }

    public WinState getWinState() {
        int dealer = dealerHand.getTotal(),
            player = playerHand.getTotal(),
            diff = player - dealer;

        if (diff == 0) return WinState.Tie;
        if (diff > 0) return WinState.PlayerWins;
        return WinState.DealerWins;
    }
}

CardValue.java

package com.stackexchange.blackjack;

public enum CardValue {
    Ace(1),
    Two(2),
    Three(3),
    Four(4),
    Five(5),
    Six(6),
    Seven(7),
    Eight(8),
    Nine(9),
    Ten(10),
    Jack(11),
    Queen(12),
    King(13);

    public final int value;

    CardValue(int value) { this.value = value; }
}

Deck.java

package com.stackexchange.blackjack;

import java.util.*;
import java.util.function.Consumer;
import java.util.stream.Collectors;

public class Deck {
    protected final List<Card> cards;

    protected Deck(List<Card> cards) {
        this.cards = cards;
    }

    public static Deck full() {
        return new Deck(
            Arrays.stream(Suit.values())
                .mapMulti(Deck::cardsInSuit)
                .collect(Collectors.toList())
        );
    }

    private static void cardsInSuit(Suit suit, Consumer<Card> consumer) {
        Arrays.stream(CardValue.values())
            .forEach(
                value -> consumer.accept(new Card(suit, value))
            );
    }

    public void shuffle() {
        Collections.shuffle(cards);
    }

    public Card topCard() {
        return cards.get(cards.size() - 1);
    }

    public void dealTo(Deck dest) {
        dest.cards.add(
            cards.remove(cards.size() - 1)
        );
    }

    @Override
    public String toString() {
        return cards.stream()
            .map(Card::toString)
            .collect(Collectors.joining(", "));
    }
}
\$\endgroup\$
2
  • \$\begingroup\$ I agree with the overall design, very solid solution which is easy to read, test and evolve. There is a couple of minor points I want to address. 1) Decks internal data store is a mutable list. Method full() makes use of Collectors.toList() to generate a list of cards. Documentation of this collector contains a word of caution in regard to mutability: "There are no guarantees on the type, mutability, serializability, or thread-safety of the List returned". \$\endgroup\$ Commented May 14 at 8:35
  • \$\begingroup\$ Collectors.toCollection() can be used instead. And by the way in JDK 21 with the introduction of SequencedCollection interface List received convenience methods getLast() and removeLast(). 2) Enum CardValue doesn't need property value, instead it can be equipped with a method int value() returning ordinal + 1. Also, according to the Java language naming convention, names of enum constants are in uppercase letters. \$\endgroup\$ Commented May 14 at 8:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.