Blackjack implementation - Java

Question

I have inspired from the recent blackjack question here and decide to code for simple blackjack console application Please review and let me know What can improve on the code?

Rules for the application:

1. There's a single dealer and a player
2. Initially both get two cards
3. Number 2 to 9 cards have their numerical value that is counted as their score, Face cards have value of 10, Ace's value changes based on the current score
4. Then Player asks dealer to hit, as long as Player's score is less than 17
5. When Player reaches 17 or more then they no longer go for hit
6. Dealer starts their turn, they go for hit as long as Dealer's score is less than 17
7. After both are done with their turns, Outcome would be decided
8. Every action has system out, output will be similar to this

Task :GamePlay.main()
Dealer got 4♣ score: 4 status: HIT
Dealer got K♦︎ score: 14 status: HIT
Player got A♦︎ score: 11 status: HIT
Player got J♥ score: 21 status: BLACKJACK
Dealer got 10♠ score: 24 status: BUST
Dealer Bust, Player Wins

class Card

public class Card {
 private Suit suit;
 private Type type;
 public Card(Suit suit, Type type) {
 this.suit = suit;
 this.type = type;
 }
 public Suit getSuit() {
 return suit;
 }
 public Type getType() {
 return type;
 }
 @Override
 public String toString() {
 return this.getType().toString() + this.getSuit().toString();
 }
}

enum Suit

public enum Suit {
 SPADE('♠'), DIAMOND('♦︎'), CLUB('♣'), HEART('♥');
 private final char symbol;
 Suit(char symbol) {
 this.symbol = symbol;
 }
 @Override
 public String toString() {
 return String.valueOf(this.symbol);
 }
}

enum Type

Type enum constructor takes two parameters, their default value and symbol to represent on output, also the enum has getDefaultValue method

Ace has overriden the method. since it has it's own implementation

import static blackjack.GamePlay.ACE_MAX_VALUE;
public enum Type {
 
 TWO(2, "2"), THREE(3, "3"), FOUR(4, "4"), FIVE(5, "5"),
 SIX(6, "6"), SEVEN(7, "7"), EIGHT(8, "8"), NINE(9, "9"),
 TEN(10, "10"),
 ACE(ACE_MAX_VALUE, "A") {
 @Override
 public int getDefaultValue(int totalValue) {
 if (totalValue + ACE_MAX_VALUE > 21) {
 return 1;
 }
 return ACE_MAX_VALUE;
 }
 },
 JACK(10, "J"), QUEEN(10, "Q"), KING(10, "K");
 
 private int defaultValue;
 private String symbol;
 Type(int defaultValue, String symbol) {
 this.defaultValue = defaultValue;
 this.symbol = symbol;
 }
 public int getDefaultValue(int totalValue) {
 return this.defaultValue;
 }
 @Override
 public String toString() {
 return this.symbol;
 }
}

class Deck

Deck has list of all 52 Cards and loads them into the list on constructor, it has getACard method which removes a card from random index and return a Card object for dealer to draw

import java.util.ArrayList;
import java.util.List;
import java.util.Random;
public class Deck {
 private List<Card> cards;
 public Deck() {
 this.cards = new ArrayList<>();
 loadCards();
 }
 private void loadCards() {
 for (Suit suit : Suit.values()) {
 for (Type type : Type.values()) {
 this.cards.add(new Card(suit, type));
 }
 }
 }
 public Card getACard() {
 int randomIndex = new Random().nextInt(this.cards.size());
 return this.cards.remove(randomIndex);
 }
}

class Dealer

Dealer has the Deck, on constructor it draws two cards and adds to their list of cards and updates their score and status.

It has hit method which draws a Card from the Deck and returns. based on the value of the card their score and status is updated.
play method does the orchestration also prints the state of the Dealer

canHit method checks the status of the Dealer returns true if they're in HIT status

import java.util.ArrayList;
import java.util.List;
import static blackjack.GamePlay.*;
public class Dealer {
 private Deck deck;
 private List<Card> cards;
 private int score;
 private Status status;
 public Dealer(Deck deck) {
 this.cards = new ArrayList<>();
 this.deck = deck;
 for (int i = 0; i < INITIAL_CARDS_COUNT; i++) {
 play();
 }
 }
 public Card hit() {
 return this.deck.getACard();
 }
 public void play() {
 Card card = hit();
 this.score += card.getType().getDefaultValue(this.score);
 this.cards.add(card);
 this.status = Status.getStatus(this.score);
 System.out.printf("Dealer got %s \t score: %d \t status: %s \n", card, this.score,this.status.toString());
 }
 public boolean canHit() {
 return this.status.equals(Status.HIT);
 }
 public Status getStatus() {
 return status;
 }
 public int getScore() {
 return score;
 }
}

class Player

Player has the Dealer, on constructor it draws two cards and adds to their list of cards and updates their score and status.

It has hit method which draws a Card from the Dealer and returns. based on the value of the card their score and status is updated.
play method does the orchestration also prints the state of the Player

canHit method checks the status of the Dealer returns true if they're in HIT status

import java.util.ArrayList;
import java.util.List;
import static blackjack.GamePlay.INITIAL_CARDS_COUNT;
import static blackjack.Status.HIT;
public class Player {
 private List<Card> cards;
 private Dealer dealer;
 private int score;
 private Status status;
 public Player(Dealer dealer) {
 this.cards = new ArrayList<>();
 this.dealer = dealer;
 for (int i = 0; i < INITIAL_CARDS_COUNT; i++) {
 play();
 }
 }
 public void play() {
 Card card = this.dealer.hit();
 this.score += card.getType().getDefaultValue(this.score);
 this.cards.add(card);
 this.status = Status.getStatus(this.score);
 System.out.printf("Player got %s \t score: %d \t status: %s \n", card, this.score,this.status.toString());
 }
 public boolean canHit() {
 return this.status.equals(HIT);
 }
 public Status getStatus() {
 return status;
 }
 public int getScore() {
 return score;
 }
}

enum Status

Status enum has constructor takes two params min and max, this decides which Status any score fall in

getStatus method takes score and return Status based on the score

public enum Status {
 HIT(0, 16), STAND(17, 20), BLACKJACK(21, 21), BUST(22, 100);
 private int min;
 private int max;
 Status(int min, int max) {
 this.min = min;
 this.max = max;
 }
 public static Status getStatus(int score) {
 Status[] statuses = Status.values();
 for (Status status : statuses) {
 if (status.min <= score && status.max >= score) {
 return status;
 }
 }
 return null;
 }
}

class GamePlay

This is orchestrator for the application creates All objects and decides the outcome of the game

public class GamePlay {
 public static final int INITIAL_CARDS_COUNT = 2;
 public static final int ACE_MAX_VALUE = 11;
 public static String getOutcome(Player player, Dealer dealer) {
 if (player.getStatus().equals(Status.BUST)) {
 return "Player Bust, Dealer Wins";
 } else if (player.getScore() == dealer.getScore()) {
 return "Tie";
 } else if (dealer.getStatus().equals(Status.BUST)) {
 return "Dealer Bust, Player Wins";
 } else if (player.getScore() > dealer.getScore()) {
 return "Player Wins";
 }
 return "Dealer Wins";
 }
 public static void main(String[] args) {
 Deck deck = new Deck();
 Dealer dealer = new Dealer(deck);
 Player player = new Player(dealer);
 while (player.canHit()) {
 player.play();
 }
 while (dealer.canHit()) {
 dealer.play();
 }
 System.out.println(getOutcome(player, dealer));
 }
}

• Ralf Kleberhoff: What is the score of ACE, THREE, KING in your program? Does that match the Blackjack rules? THREE, KING, ACE should give the same result, shouldn't it? How would you write an automated test (e.g. JUnit) for that?
• HariHaravelan: Yes that won't work out in the code, i will have to fix that @Ralf Kleberhoff

Answer 1

The code looks pretty decent. I can look at it and follow what is going on without too much difficulty. Here are a few things for your consideration. Note that some are aesthetic, and some are not super important for a small program but could be for a larger program, and there are a couple of questions to think about different ways you could have done things.

---

1. enum Type

For the Types, ACE is right after TEN and before JACK, QUEEN, and KING. Those last four all have 10 as a value, so it would probably be better to put ACE after them. (General rule of organization: Similar things go together.)

ACE_MAX_VALUE is defined over in the GamePlay class. You may have some aesthetic for putting it there, but it is only used in the Type enum. Also this creates a circular dependency through GamePlay, Deck, Type, GamePlay, etc. Consider moving it into Type, though you may have to use an ugly static nested class. (Note: You can see dependencies in a program like Stan4J or Structure101.)

The word Type is extremely generic. How about something a little more descriptive, perhaps FaceValue?

---

2. Dealer and Player

Is it necessary for the Player to have a reference to the Dealer just so the Dealer can get a card for the Player? I realize there is text in the "Rules" at the top saying that the Player gets the card from the Dealer, but it may not be necessary if you let them both know about the Deck.

The GamePlay class has the logic for while (player.canHit()) player.play();. Could Player simply have a playAll() method?

These two classes are nearly identical, so there is duplicate code. Could they be the same? If so, you could move the definition of the INITIAL_CARDS_COUNT constant into that class. (General principle: DRY - Don't Repeat Yourself)

---

3. Deck

The Deck is creating a new Random() for each draw. You could create one, and save it as part of the class state. This is probably not important for such a small program.

Something else that may not be important in this example, when you remove a card from the List, all other cards, all other cards from the removed index to the end of the list have to be moved down. Mind you, your code is clear and correct as is, however it is just a little inefficient. You could read the item at the index, move the last item to the current item, and then remove the final item from the List so nothing gets moved down.

The method name getACard() might read better as drawCard().

---

4. Have you considered unit testing?

How would you unit test this thing? How many unit tests would you need? Perhaps you could have an alternate constructor for Deck that takes in a seed for the Random to get a predictable order.

How would the program have come out different if you had coded it with TDD (or even BDD)? Just in case you (or other readers) don't know, JUnit is the most common Java TDD framework, and Cucumber for Java is the most common Java BDD framework.

---

5. enum Status

The game logic only depends on canHit, so you really only need two Statuses - one for playing and one for done. Or perhaps play() can return a boolean indicating whether it is done? Perhaps there should be a separate enum Result?

---

Nice work. I hope this gives you some ideas to "plus" it.

• HariHaravelan: Your suggestions are helpful, very much appreciated These two classes are nearly identical, so there is duplicate code. Could they be the same, how do you suggest i do that inheritance?
• Pixel: You could potentially avoid inheritance completely by having only one class. If you just have a Player class, you can say Player player1 = new Player("Player", deck); Player dealer = new Player("Dealer", deck);. The only thing that needs to differ is the name, so pass that into the constructor. Interfaces or abstract classes would be overkill. A single class would be the simplest thing you could do.

Answer 2

Testing

• There is none.

Type

• I would expect this class to be named Rank, which is what this concept is called by card players.

• The value of a card doesn't belong in this class, because the value of cards varies by game. This implementation restricts the reusability of a Type to only blackjack.

Deck

• This implementation is confusing. A deck of cards is shuffled, and then during the course of play the top card is drawn. That would be just as easy to implement and would make more sense to people familiar with card games than pulling a random card from somewhere in the deck.

• When working with Random, it's important to pass in (inject) an instance rather than instantiating it inside the class. The reason is repeatable unit tests - if the test can't control the random seed, deck behavior can't be predicted.

Dealer

• This class is overloaded .. it's doing two things. It is both a player of the game and a distributor of cards. The Player class, should be used to model the "player of the game" aspect. This will also cut down on repeated code.

• The computation of hand value ("score") does not work correctly when the first card drawn is an ace. Its value will always be 11. If the first card is not an ace but the second card is, its value will always be 11.

Player

• Player should be an interface, with perhaps an abstract class holding supporting code. A ComputerDealer might be one type of player. A ConsoleHuman might be another type, where information is written to and read from the console to allow actual play. Players should have a hand of cards, and should respond to a method call by reporting how they act (hit or stand). If they hit, the controller should give the player another card and then check the value of their hand to see if they've busted.

• As it is, there are two robots with the same algorithm "playing" each other, but they're in different classes with duplicated code.

Status

• This isn't status, this is the logic for determining how a computer player should behave, loosely corresponding to the perceived value of their hand. A more descriptive name would be better. But really, this logic belongs in ComputerDealer. And to hold to casino rules, it should address soft vs. hard 17s.

GamePlay

• ACE_MAX_VALUE belongs in the new class that will compute hand value.

• The rules on how to play blackjack, which are scattered through the other classes, should be pulled into this class.

• HariHaravelan: Suggestions are helpful, Really appreciated.