I've created a relatively simple Blackjack game in java. The reason why I decided to do this specific project was to improve my object orientated programming in java.
I will post my code so feel free to come with criticism etc. I am reading my first course in Java, have that in mind.
Player class
import java.util.ArrayList;
public class Player {
private String nickName;
private int playerNumOfCards;
ArrayList<Card> playerHand;
public Player (String name){
this.nickName = name;
playerHand = new ArrayList<Card>();
}
public String getNickName() {
return nickName;
}
public void addCard(Card aCard){
playerHand.add(aCard);
this.playerNumOfCards++;
}
public int getHandSum(){
int totalSum = 0;
for(Card countSum: playerHand){
totalSum = totalSum + countSum.getFace();
}
return totalSum;
}
public void getPlayerHand(boolean hideCard) {
System.out.println(this.nickName + "s current hand.");
for ( int c = 0; c < playerNumOfCards; c++){
if(c == 0 && !hideCard){
System.out.println("[Hidden card]");
} else {
System.out.println(playerHand.get(c).toString());
}
}
}
}
Card class
public class Card {
private Face face; //Face of card, i.e "King" & "Queen"
private Suit suit; //Suit of card, i.e "Hearts" & "diamonds"
int total = 0;
public Card (Face cardFace, Suit cardSuit){ //Constructor which initializes card's face and suit
this.face = cardFace;
this.suit = cardSuit;
}
public int getFace(){
return face.getFaceValue();
}
public String getSuit(){
return suit.PrintSuitText();
}
public String toString(){ //return String representation of Card
return face + " of " + suit;
}
}
Suit enum
public enum Suit {
HEARTS(" Hearts"),
SPADES(" Spades"),
DIAMONDS(" Diamonds"),
CLUBS(" Clubs");
private final String suitText;
private Suit(String suitText){
this.suitText = suitText;
}
public String PrintSuitText(){
return suitText;
}
}
Face enum
public enum Face {
ACE(1), DEUCE (2), THREE (3),
FOUR(4), FIVE(5), SIX(6),
SEVEN(7), EIGHT(8), NINE(9),
TEN(10), JACK(10), QUEEN(10),
KING(10);
private final int faceValue;
private Face(int faceValue){
this.faceValue = faceValue;
}
public int getFaceValue(){
return faceValue;
}
}
DeckOfCard class
import java.util.Random;
public class DeckOfCards {
private Card[] deck;
private static final Random random = new Random();
private int currentCard; //index of next Card to be deal (0-51)
private static int NUMBER_OF_CARDS = 52; //Constant number of cards
public DeckOfCards(){
Face [] faces = {Face.ACE, Face.DEUCE, Face.THREE, Face.FOUR, Face.FIVE, Face.SIX,
Face.SEVEN, Face.EIGHT, Face.NINE, Face.TEN, Face.JACK, Face.QUEEN,
Face.KING};
Suit[] suits = {Suit.HEARTS, Suit.SPADES, Suit.DIAMONDS, Suit.CLUBS};
deck = new Card [NUMBER_OF_CARDS]; // create array with Cards (52)
currentCard = 0;
//Populate deck with Cards
for(int count = 0; count < deck.length; count++)
deck [count] = new Card(faces [count % 13], suits [count / 13]);
}
public void shuffleDeck(){
currentCard = 0;
for (int first = 0; first < deck.length; first++){
int second = random.nextInt(NUMBER_OF_CARDS); //Select a random card from number 0-51 (Number_of_cards)
//Loops through all the cards and swaps it with the "Second" card which is randomly chosen card from hte same list.
Card temp = deck[first];
deck [first] = deck [second];
deck [second] = temp;
}
}
public void getCardDeck(){
int start = 1;
for(Card k : deck) {
System.out.println("" + start + "/52 " + k);
start++;
}
}
public Card dealNextCard(){
//Get the top card
Card topCard = this.deck[0];
//shift all the subsequent cards to the left by one index
for(int currentCard = 1; currentCard < NUMBER_OF_CARDS; currentCard ++){
this.deck[currentCard-1] = this.deck[currentCard];
}
this.deck[NUMBER_OF_CARDS-1] = null;
//decrement the number of cards in our deck
this.NUMBER_OF_CARDS--;
return topCard;
}
}
Main class
import java.util.Scanner;
public class BlackJackGame {
public static void main(String[] args) {
Scanner scanner = new Scanner(System.in);
boolean stay = false;
System.out.println("What nickName would you like to have?");
String pAnswer = scanner.nextLine();
Player me = new Player(pAnswer);
Player dealer = new Player("Dealer");
System.out.println("Would you like to start a new game? 'Yes/No' :");
pAnswer = scanner.nextLine();
if (pAnswer.equalsIgnoreCase("Yes")) {
DeckOfCards deck1 = new DeckOfCards();
Card card1 = new Card(Face.ACE, Suit.CLUBS);
deck1.shuffleDeck();
me.addCard(deck1.dealNextCard());
me.addCard(deck1.dealNextCard());
me.getPlayerHand(true);
System.out.println(" ");
dealer.addCard(deck1.dealNextCard());
dealer.addCard(deck1.dealNextCard());
dealer.getPlayerHand(false);
//PLAYER
do {
System.out.println("Would " + me.getNickName() + " like to bust or stay? 'Bust/Stay'");
pAnswer = scanner.nextLine();
//BUST
if (pAnswer.equalsIgnoreCase("Bust")) {
me.addCard(deck1.dealNextCard());
System.out.println(me.getHandSum());
if (me.getHandSum() > 21) {
System.out.println("You busted and got a total of " + me.getHandSum() + ". Dealer wins this time! ");
System.exit(0);
}
}
//STAY
if (pAnswer.equalsIgnoreCase("stay")) {
System.out.println("You have chosen to stay. Your hand: " + me.getHandSum());
}
} while (pAnswer.equalsIgnoreCase("Bust"));
//DEALER
stay = false;
do {
System.out.println("");
System.out.println("- Dealers turn -");
//DRAW CARD
if (dealer.getHandSum() <= 15) {
dealer.addCard(deck1.dealNextCard());
if(dealer.getHandSum() == 15){
System.out.println("Blackjack! Dealer won.");
System.exit(0);
}
if (dealer.getHandSum() > 21) {
System.out.println("Dealer busted and got a total of " + dealer.getHandSum() + ". " + me.getNickName() + " wins this time!");
System.exit(0);
}
} else {
System.out.println("Dealer has chosen to stay!");
int totalDealerSum = dealer.getHandSum();
int totalPlayerSum = me.getHandSum();
if(totalDealerSum > totalPlayerSum){
System.out.println("Both players has decided to stay. The winner is " + dealer.getNickName() + " with a total of " + totalDealerSum + ".");
} else {
System.out.println("Both players has decided to stay. The winner is " + me.getNickName() + " with a total of " + totalPlayerSum + ".");
}
stay = false;
}
} while (stay);
}
}
}
4 Answers 4
This is an example run that shows that the rules of the game are not being followed. If I started with 5♣︎ and 5♥, then receive an Ace, then I've achieved 21. I should have won.
Also note that "bust or stay" is not the right terminology. Nobody wants to bust. It should ask whether I want to "hit or stay".
$ java BlackJackGame
What nickName would you like to have?
200_success
Would you like to start a new game? 'Yes/No' :
Yes
200_successs current hand.
FIVE of CLUBS
FIVE of HEARTS
Dealers current hand.
[Hidden card]
TEN of HEARTS
Would 200_success like to bust or stay? 'Bust/Stay'
Bust
11
Would 200_success like to bust or stay? 'Bust/Stay'
Stay
You have chosen to stay. Your hand: 11
- Dealers turn -
Dealer has chosen to stay!
Both players has decided to stay. The winner is Dealer with a total of 20.
Overall, the quality of the code is very good. There is good breakdown of the code into classes and methods, good utilization of OO paradigms like data abstraction, proper use of features like enums.
Here are my comments:
Player class
getPlayerHand()
the naming convention dictates that this method should have return something but it doesn't. it protins to stdout aString
representation of the class. so perhaps the nameprintPlayerHand()
is more appropriate. However, best practice convention is for a class to provide aString
representation by overridingtoString()
method and let the caller decide what to do with thatString
(maybe print to stderr? maybe send to UI? many choices)getHandSum()
it is just too pretty a name...
Card class
getFace()
the name suggests that this getter method returns aFace
instance, but it returns a property of theFace
instance variable. so either rename togetFaceValue()
or return theFace
instance variable and let the caller chain method calls to get the value:getFace().getFaceValue()
. I would go with 2nd option since it allows caller to get whatever they wish fromFace
instance variable.total
seems like leftover from abandoned development...getSuit()
same comment asgetPlayerHand()
DeckOfCard class
in the constructor, you populate the deck and use the magic number 13. it is the length of the faces array. althouth it is unlikely that a new face will be introduced anytime in the future, still it is good for clarity to erplace the literal with a calculated number
shuffleDeck()
it is ok to do the shuffling for practice. However, you should know that Java has built-in feature to shuffle collections (likeList
and such)
-
\$\begingroup\$ Actually the code is very poor because of all those getters. \$\endgroup\$Martin Spamer– Martin Spamer2018年10月09日 16:39:08 +00:00Commented Oct 9, 2018 at 16:39
Review
Code does not work as intended
I have no idea if you uploaded your latest code, but this one here doesn't seem to work. Actually that's a reason to close this topic:
Code not implemented or not working as intended: Code Review is a community where programmers peer-review your working code to address issues such as security, maintainability, performance, and scalability. We require that the code be working correctly, to the best of the author's knowledge, before proceeding with a review.
I made this review for you anyways, but keep this in mind for future questions. I uploaded a working version of your code below.
Java Conventions
Good Job.
Close Scanner
Maybe you saw the waring java gave you, that your scanner is not closed at all. In this example it doesn't make a difference, but it's good practice.
Naming
Some fields seems to have redundant names. You could rename some:
Player.playerNumOfCards
>Player.numOfCards
Player.playerHand
>Player.hand
Suit.suitText
>Suit.text
Obsolete code
Defining Enum.values() yourself
There is no need for
Face [] faces = {Face.ACE, Face.DEUCE, Face.THREE, Face.FOUR, Face.FIVE, Face.SIX,
Face.SEVEN, Face.EIGHT, Face.NINE, Face.TEN, Face.JACK, Face.QUEEN,
Face.KING};
Suit[] suits = {Suit.HEARTS, Suit.SPADES, Suit.DIAMONDS, Suit.CLUBS};
Since there are Face.values()
and Suit.values()
, which will return exactly those arrays.
No need fo Suits.suitText
As I have been reworking your code, I noticed that you don't need this field at all.
Replace DeckOfCards.deck by an ArrayList
Doing so saves you alot of trouble. See example
Use as many private fields as possible
Actually your can make all of your private
fields final
. Good practice.
You may remove the getters of final fields
Since msot of your fields are final, you may remove the getters and make the fields prublic instead. More information: https://softwareengineering.stackexchange.com/questions/275691/is-public-final-completely-not-ok-in-constant-enum-fields
NUMBER_OF_CARDS is kinda weird
NUMBER_OF_CARDS
is supposed to be a class constant, but at some point it seems like you decided to make a dirty workaround happen. I fixed this in my example.
Don't use System.out.print inside a method
Just don't. Instead return a String, which can be printed by whoever called your method. Helpful for building long Strings: https://docs.oracle.com/javase/7/docs/api/java/lang/StringBuilder.html
Code example
Card.java
public class Card {
private final Face face;
private final Suit suit;
public Card(Face face, Suit suit) {
this.face = face;
this.suit = suit;
}
public Face getFace() {
return face;
}
public Suit getSuit() {
return suit;
}
@Override
public String toString() {
return face + " of " + suit;
}
}
Deck.java
import java.util.ArrayList;
import java.util.Collections;
public class Deck {
private final ArrayList<Card> cards;
public Deck() {
cards = new ArrayList<Card>();
// populate deck with cards
for (Suit suit : Suit.values()) {
for (Face face : Face.values()) {
cards.add(new Card(face, suit));
}
}
}
public void shuffle() {
Collections.shuffle(cards);
}
public Card draw() {
return cards.remove(0);
}
@Override
public String toString() {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < cards.size(); i++) {
sb.append(i + 1);
sb.append('/');
sb.append(cards.size());
sb.append(' ');
sb.append(cards.get(i));
sb.append('\n');
}
return sb.toString();
}
}
Face.java
public enum Face {
ACE(1), DEUCE(2), THREE(3), FOUR(4), FIVE(5), SIX(6), SEVEN(7), EIGHT(8), NINE(9), TEN(10), JACK(10), QUEEN(
10), KING(10);
private final int value;
private Face(int value) {
this.value = value;
}
public int getValue() {
return value;
}
}
Player.java
import java.util.ArrayList;
public class Player {
private final String nickname;
private final ArrayList<Card> hand;
public Player(String nickname) {
this.nickname = nickname;
this.hand = new ArrayList<Card>();
}
public String getNickname() {
return nickname;
}
public void addCard(Card card) {
hand.add(card);
}
public int getHandSum() {
int handSum = 0;
for (Card card : hand) {
handSum += card.getFace().getValue();
}
return handSum;
}
public String getHandAsString(boolean hideCard) {
StringBuilder sb = new StringBuilder();
sb.append(nickname + "\'s current hand:");
sb.append('\n');
for (int i = 0; i < hand.size(); i++) {
if (i == 0 && hideCard) {
sb.append("[Hidden card]");
sb.append('\n');
} else {
sb.append(hand.get(i));
sb.append('\n');
}
}
return sb.toString();
}
}
Suit.java
public enum Suit {
HEARTS, SPADES, DIAMONDS, CLUBS;
}
BlackJackGame.java
import java.util.Scanner;
public class BlackJackGame {
public static void main(String[] args) {
Scanner scanner = new Scanner(System.in);
String nickname;
String input;
// ask for nickname
System.out.println("What nickname would you like to have?");
input = scanner.nextLine();
nickname = input;
// mainloop
do {
// new game message
System.out.println();
System.out.println("A new game has begun.");
System.out.println();
// init
Player player = new Player(nickname);
Player dealer = new Player("Dealer");
Deck deck = new Deck();
deck.shuffle();
boolean gameOver = false;
// give cards to the player
player.addCard(deck.draw());
player.addCard(deck.draw());
System.out.println(player.getHandAsString(false));
// give cards to the dealer
dealer.addCard(deck.draw());
dealer.addCard(deck.draw());
System.out.println(dealer.getHandAsString(true));
// player's turn
do {
System.out.println("Would " + player.getNickname() + " like to bust or stay? 'Bust/Stay'");
do {
input = scanner.nextLine();
} while (!input.equalsIgnoreCase("Bust") && !input.equalsIgnoreCase("Stay"));
// BUST
if (input.equalsIgnoreCase("Bust")) {
player.addCard(deck.draw());
System.out.println(player.getNickname() + " drew a card.");
System.out.println();
System.out.println(player.getHandAsString(false));
if (player.getHandSum() > 21) {
System.out.println(
"You busted and got a total of " + player.getHandSum() + ". Dealer wins this time!");
gameOver = true;
}
}
// STAY
if (input.equalsIgnoreCase("stay")) {
System.out.println("You have chosen to stay. Your hand: " + player.getHandSum());
}
} while (input.equalsIgnoreCase("Bust") && !gameOver);
// dealer's turn
if (!gameOver) {
System.out.println();
System.out.println("- Dealers turn -");
System.out.println();
System.out.println(dealer.getHandAsString(false));
}
while (!gameOver) {
if (dealer.getHandSum() <= 15) {
// DRAW CARD
dealer.addCard(deck.draw());
System.out.println(dealer.getNickname() + " drew another card");
System.out.println();
System.out.println(dealer.getHandAsString(false));
if (dealer.getHandSum() == 15) {
System.out.println("Blackjack! Dealer won.");
gameOver = true;
}
if (dealer.getHandSum() > 21) {
System.out.println("Dealer busted and got a total of " + dealer.getHandSum() + ". "
+ player.getNickname() + " wins this time!");
gameOver = true;
}
} else {
// STAY
System.out.println("Dealer has chosen to stay!");
System.out.println();
int totalDealerSum = dealer.getHandSum();
int totalPlayerSum = player.getHandSum();
if (totalDealerSum > totalPlayerSum) {
System.out.println("Both players has decided to stay. The winner is " + dealer.getNickname()
+ " with a total of " + totalDealerSum + ".");
} else {
System.out.println("Both players has decided to stay. The winner is " + player.getNickname()
+ " with a total of " + totalPlayerSum + ".");
}
gameOver = true;
}
}
// ask for new game
System.out.println();
System.out.println("Would you like to start a new game? 'Yes/No' :");
do {
input = scanner.nextLine();
} while (!input.equalsIgnoreCase("Yes") && !input.equalsIgnoreCase("No"));
} while (input.equalsIgnoreCase("Yes"));
// tidy up
scanner.close();
}
}
-
\$\begingroup\$ If the code is obviously not working correctly, please don't answer the question. Flag it for closure instead. \$\endgroup\$200_success– 200_success2018年10月09日 18:24:30 +00:00Commented Oct 9, 2018 at 18:24
One more point on OOP:
You hold your deck in an array. Because of this you need to "move" the cards in the deck after getting the top most.
You may better use a Collection
type to hold them. Since order is important you man use an implementation of List
. Then the whole method dealNextCard()
would reduce to:
private final List<Card> deck = new ArrayList<>();
public Card dealNextCard(){
return this.deck.remove(0);
}
This also would remove the need to track the number of cards left since you can call isEmpty()
on any Collection
.
Unfortunately this code has a magic number which should be replaced by a constant with a meaningfull name:
private static final int TOP_POSITION = 0;
private final List<Card> deck = new ArrayList<>();
public Card dealNextCard(){
return this.deck.remove(TOP_POSITION);
}
Alternatively you could use a LinkedList
object which provides the method removeFirst()
:
private final LinkedList<Card> deck = new LinkedList<>();
public Card dealNextCard(){
return this.deck.removeFirst();
}
This last version needs to be well thought since it violates the program against interfaces paradigm.
Explore related questions
See similar questions with these tags.
Face.values()
andSuit.values()
? \$\endgroup\$