I wrote this blackjack game for my APCSA class (already submitted) and was wondering if anyone had feedback on what parts of it are improperly written. My main concern is the fact that I am printing from my object bodies, and I don't know if that is bad practice.
Additionally, does anyone have an idea of how I would go about prompting the user to enter the value of the aces he draws and preventing duplicate cards? I overrode the .equals
method already but couldn't figure out how to properly making an ArrayList that could use .contains on my card objects. I am fine with solutions that have hard concepts in them as I am looking to expand into more advanced areas.
Github link: https://github.com/nohgo/blackjackProject
Card Class
import java.util.*;
public class Card {
Scanner input2 = new Scanner(System.in);
private int cardFace;
private int cardSuit;
private int value;
static private String[][] faces = {{"2","2"}, {"3","3"}, {"4","4"}, {"5","5"}, {"6","6"}, {"7","7"}, {"8","8"}, {"9","9"},
{"10","10"}, {"Jack","10"}, {"Queen","10"}, {"King","10"}, {"Ace", "11"}};
static private String[] suits = {"Spades", "Clubs", "Hearts", "Diamonds"};
public Card() { // only constructor
cardFace = (int)(Math.random() * faces.length);
cardSuit = (int)(Math.random() * suits.length);
value = Integer.valueOf(faces[cardFace][1]);
}
public int getValue() {
return value;
}
public String getFace() {
return faces[cardFace][0];
}
public String getSuit() {
return suits[cardSuit];
}
@Override
public String toString() {
return this.getFace() + " of " + this.getSuit();
}
@Override
public boolean equals(Object x) {
if (x == null) return false;
if (x == this) return true;
if (!(x instanceof Card)) return false;
Card card = (Card) x;
return (this.getFace() == card.getFace() && this.getSuit() == card.getSuit());
}
}
Individual Class
import java.util.*;
public abstract class Individual {
private int total;
private ArrayList<Card> hand = new ArrayList<Card>();
public String hit() {
hand.add(new Card());
total += (hand.get(hand.size() - 1)).getValue();
return (hand.get(hand.size() - 1)).toString();
}
public int getTotal() {
return total;
}
public void clear() {
hand.clear();
total = 0;
}
public String listHand() {
ArrayList<String> handString = new ArrayList<String>();
hand.forEach(n -> handString.add(n.toString()));
return (String.join(", ", handString));
}
public boolean isBust() {
return total > 21;
}
}
Player Class
public class Player extends Individual {
private int chipCount;
private static final int DEFAULT_CHIP = 2500;
public Player() {
chipCount = DEFAULT_CHIP;
}
public void loseChips(int lost) {
chipCount = Math.subtractExact(this.getChipCount(), lost);
}
public void gainChips(int gain) {
chipCount = Math.addExact(this.getChipCount(), gain);
}
public int getChipCount() {
return chipCount;
}
public String evaluateWinner(Individual dealer, int bet) {
if (this.isBust()) {this.loseChips(bet); return("You bust. Dealer win."); }
else if (dealer.isBust()) {this.gainChips(bet); return("Dealer bust. You win."); }
else if (dealer.getTotal() > this.getTotal()) {this.loseChips(bet); return("Dealer wins."); }
else if (dealer.getTotal() < this.getTotal()) {this.gainChips(bet); return("You win.");}
else if (dealer.getTotal() == this.getTotal()) return("Tie.");
return "WRONG";
}
}
Dealer class
public class Dealer extends Individual{
private final int DEFAULT_HIT = 17;
public void stand() {
while(this.getTotal() < DEFAULT_HIT) {
System.out.println("Dealer drew a " + this.hit() + ".");
}
}
}
Application class
package nohBlackjack;
import java.util.*;
public class BlackjackMain {
public static void main(String[] args) {
Scanner input = new Scanner(System.in);
Player player = new Player();
Dealer dealer = new Dealer();
System.out.println("Welcome to BlackJack.\nDealers hit until 17.\nAces are worth 11. ");
System.out.println();
while(true) {
int bet;
//bet loop
while (true) {
System.out.print("You currently have " + player.getChipCount() + " chips. \nHow much would you like to bet? ");
bet = input.nextInt();
if(bet <= player.getChipCount()) break;
}
input.nextLine();
System.out.println();
//resetting both people
player.clear();
player.hit();
player.hit();
dealer.clear();
dealer.hit();
System.out.println("You have a " + player.listHand());
System.out.println("Dealer has a " + dealer.listHand());
System.out.println();
//main loop
while(true) {
System.out.print("Hit or stand? ");
String currentInput = input.nextLine().toLowerCase();
if (currentInput.equals("hit")) {
System.out.println("You got a " + player.hit());
if (player.isBust()) {
break;
}
}
else if (currentInput.equals("stand")) {
dealer.stand();
break;
}
else {
System.out.println("Input not recognized.");
}
}
System.out.println();
System.out.println("Player total: " + player.getTotal() + ". Dealer total: " + dealer.getTotal() + ".");
//win checking
System.out.println(player.evaluateWinner(dealer, bet));
System.out.println();
System.out.println("New chip count: " + player.getChipCount());
//broke check
if(player.getChipCount() == 0) {
System.out.println("You are out of chips.");
break;
}
//replay?
System.out.print("Do you want to play again? (y or n) ");
String replay = input.nextLine().toLowerCase();
if (replay.equals("n")) {System.out.println("\nQuitting... "); break;}
else System.out.println("\n" + ("- ").repeat(50) + "\n");
}
input.close();
System.exit(0);
}
}
1 Answer 1
The main problem with the randomization is that every card is taken from a full deck of cards. Whenever a new card is created, there is a 1/52 chance that you get the ace of spades, or any other card. The correct way to do it is to introduce the concept of Deck
. When the deck is initialized, you create all those 52 cards in it and shuffle them. When a card is dealt to an individual, you remove it from the deck so that another individual cannot receive the same card.
Individual
The hit()
method is responsible for creating the random card. In a domain driven application, this would be the responsibility of the dealer
. Because the dealer has two responsibilities: the dealer playing on behalf of the house and the dealer dealing the cards, you might want to introduce a CardDealer
class. The card dealer would be responsible for handling the Deck
.
The listHand()
seems to do the equivalent of toString()
. Might as well call it that since you're also using Card.toString()
. And if you use Streams there is no need to create a separate list:
public String toString() {
return hand.stream()
.map(Card::toString)
.collect(Collectors.joining(", "));
}
Player
The DEFAULT_CHIP
constant defines the initial amount of chips, so rename it to INITIAL_CHIP_COUNT
. Since there is no way to define any other initial chip count, calling it default is a bit misleading.
Using Math.subtractExact(int, int)
in loseChips
doesn't really do anything useful, since the game does not allow the player to get into debt. It only throws an exception if the result is smaller than Integer.MIN_VALUE. If a sanity check is needed here, you should just check that the result does not become negative. The game engine should ensure that the player cannot bet more chips that they are holding. Similar with the gainChips
. While "crashing and burning the casino when the player wins more than two billion chips" does in fact follow domain driven design, it would probably be best to just congratulate the player for winning the game. :)
The evaluateWinner
does not belong in the Individual
class. The dealer does not ask a player if they have won. The dealer tells it to the players. I would create a class for the GameRules
that can tell if an individual has won, busted or if the dealer should stand etc. and call that methond from the game loop.
Main loop
Considering the printing from the Dealer
class. You should separate the decision making from the actual operation of drawing a card. You need a method that gives a boolean for whether the dealer wants to hit or pass. This should be an abstract method in the Individual class so that the player and the dealer can have different strategies.
public boolean shouldHit() {
return total < 17;
}
And if you implement the GameRules
class, the comparison in the method should be deferred to that class. Something like return gameRules.shouldDealerHit(currentTotal)
. Then in the main loop you just hit cards until an individual passes:
while (individual.shouldHit()) {
Card card = cardDealer.dealCard();
System.out.printf("%s drew a %s", individual.getName(), card.toString());
individual.hit(card);
}
After every player has stopped drawing cards, you evaluate the winner and distribute the winnings.
Having a separate CardDealer
and a Deck
may be unnecessary complication for you. I have done it this way because it allows you to separate the deck from dealing the cards. You could then implement different decks (if you're not playing with a full deck, for example) or implement a dishonest dealer simply by swapping the Deck
or CardDealer
to another implementation.
Here is a good read for improving software design: https://en.wikipedia.org/wiki/Single_responsibility_principle
Explore related questions
See similar questions with these tags.
List<String> handString = new ArrayList<>();
(It allows dealing with algorithms on a broader interface level, and reimplement with an other class likeLinkedList
.)System.exit(0);
is not needed, a C relict. \$\endgroup\$Card
class that createsnew Card()
instances for each card in a deck may not be needed, since cards are just sequences of numbers. I would think a class makes more sense if each card instance has its own state. (for exampleflipped = true
). Maybe a very simple array is enough?String[] deck = {"Hearts8", "Hearts9, "Hearts10", "HeartsJack", ...};
etc. Shuffle the array and start drawing cards from the top! \$\endgroup\$