Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am not happy with this code as I am sure there are better ways to do what I'm trying to achieve. I'm a beginner and I've used what I know to date to complete this.

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


public class SeventySix {

    public static void main(String[] args) {

        Scanner keyboard = new Scanner(System.in);

        //52 Cards, Aces = 11, Picture cards = 10, Ace's cannot be reduced to 1.
        int[] newCard = {2,2,2,2,3,3,3,3,4,4,4,4,5,5,5,5,6,6,6,6,7,7,7,7,8,8,8,8,9,9,9,9,10,10,10,10,10,10,10,10,10,10,10,10,10,10,10,10,11,11,11,11};

        //Shuffle. Once per game.
        shuffleArray(newCard);

        //Start BlackJack.
        System.out.println("Welcome to BlackJack!");
        System.out.println();
        System.out.println("You get a " + newCard[0] + " and a " + newCard[1] + ".");
        int playerTotal = newCard[0] + newCard[1];
        System.out.println("Your total is " + playerTotal + ".");
        System.out.println();

        //Player can get blackjack/bust in the 1st deal. - awaiting betting system (enhanced bets for blackjack in first round)
        if (playerTotal == 21){
            System.out.println("Blackjack, you win.");
            System.exit(0);
        }
        if (playerTotal > 21){
            System.out.println("Bust, You lose.");
            System.exit(0);
        }
        // Dealer cards
        System.out.println("The dealer has a " + newCard[2] + " showing, and a hidden card.");
        int dealerTotal = newCard[2] + newCard[3];
        if (dealerTotal > 21){     //Dealer bust check.
            System.out.println();
            System.out.println("Dealers total is " + dealerTotal + ".");
            System.out.println("Dealer is bust, you win!");
            System.exit(0);
        }
        if (dealerTotal == 21){    //Dealer blackjack check.
            System.out.println();
            System.out.println("Dealer reveals his second card: " + newCard[3] + ".");
            System.out.println("Dealers total is " + dealerTotal + ".");
            System.out.println();
            System.out.println("Dealer has BlackJack, you lose.");
            System.exit(0);
        }
        System.out.println("His total is hidden.");
        System.out.println();


        // Hit or Stay for player.
        System.out.print("Would you like to \"hit\" or \"stay\"? ");
        String hitStay = keyboard.next();
        System.out.println();

        //cc = card count
        int cc = 4; 
        if (hitStay.equalsIgnoreCase("hit")){
            // While loop to ensure different cards & multiple "hits".
            while (playerTotal < 21 && hitStay.equalsIgnoreCase("hit")){
                if (hitStay.equalsIgnoreCase("hit")){
                    System.out.println("You drew a " + newCard[cc] + ".");
                    playerTotal = playerTotal + newCard[cc];
                    System.out.println("Your total is " + playerTotal + ".");
                    System.out.println();
                    cc++;        //Adds 1 to ensure next card is different.
                    // Bust & Blackjack check.
                    if (playerTotal > 21){
                        System.out.println("You are bust, You lose.");
                        System.exit(0);
                    }
                    if (playerTotal == 21){
                        System.out.println("Blackjack, you win.");
                        System.exit(0);
                    }
                    System.out.print("Would you like to \"hit\" or \"stay\"? ");
                    hitStay = keyboard.next();
                    System.out.println();
                }
            }        
        }

        // Dealers turn, only if Round 1 didn't end in bust/blackjack.
        keyboard.close();
        System.out.println("Ok dealers turn.");
        System.out.println("His hidden card was a " + newCard[3] + "."); // reveal hidden from round one.

        cc++; // Pretty sure its not needed.
        while (dealerTotal < 16){ // Dealer will stay on 16+ and hit if below.
            System.out.println();
            System.out.println("Dealer chooses to hit.");
            System.out.println("He draws a " + newCard[cc] + ".");
            cc++;
            dealerTotal = dealerTotal + newCard[cc];
            System.out.println();
            System.out.println("His total is " + dealerTotal);
            // bust check - no need for blackjack check due to final win sequence
            if (dealerTotal > 21){
                System.out.println();
                System.out.println("Dealer is bust, YOU WIN!");
                System.exit(0);
            }
            // stay condition.
            if (dealerTotal < 21 && dealerTotal > 16){
                System.out.println();
                System.out.println("Dealer Stays.");
            }
        }

        // final win sequence.
        System.out.println();
        System.out.println("Dealer total is " + dealerTotal);
        System.out.println("Your total is " + playerTotal);
        System.out.println();

        if (dealerTotal > playerTotal){
            System.out.println("Dealer wins.");
        } 
        if (dealerTotal == playerTotal){
            System.out.println("You both draw.");
        }
        if (dealerTotal < playerTotal){
            System.out.println("You win.");
        }
    }

    static void shuffleArray(int[] deckCards){

        /**
         * This code is obtained from the internet and is not my own though process
         * I need to understand it before I will be happy using it.
         * I'll have a try at explaining this, please correct me if I suckarino.
         * deckCards is a placeholder for the array I chose to use.
         * i = the length of the array which is 52, -1 would be 51.
         * i will be greater than 0 until the i-- completely loops it down to 0.
         * index is a random number between 1 & 52.
         * a is a random number in the array.
         * deckCards[i] is replaced with a; 
         * essentially shuffling 1 card in the array, this happens 51 times?
         */

        Random rnd = new Random();
        for (int i = deckCards.length - 1; i > 0; i--)
        {
            int index = rnd.nextInt(i + 1);
            // Swap
            int a = deckCards[index];
            deckCards[index] = deckCards[i];
            deckCards[i] = a;
        }
    }
}

I've tried to keep the comments understandable to some degree.

share|improve this question

4 Answers 4

up vote 14 down vote accepted

You should make your entire application more Object Oriented. A good place to start would be to move the deck of cards and associated functions into other classes.

Here's an implementation of a deck stolen from a StackOverflow Question. Notice there's also a Card object.

public class DeckOfCards {
  private Card cards[];

  public DeckOfCards() {
    this.cards = new Card[52];
    for (int i = 0; i < 52; i++) {
        Card card = new Card(...); //Instantiate a Card
        this.cards[i] = card; //Adding card to the Deck
    }
  }
}

Also think about using enums for suit and a method of tracking the card value. Searching Google for Cark rank patterns will help with this.

You have a lot of multiline print statements that could probably be joined together;

System.out.println();
System.out.println("Dealer total is " + dealerTotal);
System.out.println("Your total is " + playerTotal);
System.out.println();

Could be;

System.out.println(
  String.format("\nDealer total is %s \nYour total is %s \n", dealerTotal, playerTotal)
);

To save on complexity, these should be else ifs instead;

if (dealerTotal > playerTotal){
    System.out.println("Dealer wins.");
} 
else if (dealerTotal == playerTotal){
    System.out.println("You both draw.");
}
else {
    System.out.println("You win.");
}

Finally, your code is not very DRY. You need more of your functionality in separate methods to prevent you repeating yourself. A good example of this is that you have multiple win sequences. Your win/lose conditions should result in a win/lose method being called which executes the appropriate sequence.

Also Blackjack is a repetitious game, turn two is identical to turn one, your code just repeats the same operations a second time. Again this should be moved into methods.

share|improve this answer

I'd just like to point out that though the cards would be better represented as objects instead of ints, and the program organization and flow is a bit muddled, your choice of shuffling algorithm is excellent.

The Fisher-Yates shuffle is a simple and unbiased way to randomize the order of the elements of an array, and you have a textbook implementation of it. Even if you didn't write the code yourself, I think you should get credit for picking the right code to copy.


Edit: Since you asked about the validity of the comment in shuffleArray()

This is what you wrote, ignoring the initial noise:

  • deckCards is a placeholder for the array I chose to use.
  • i = the length of the array which is 52, -1 would be 51.
  • i will be greater than 0 until the i-- completely loops it down to 0.
  • index is a random number between 1 & 52.
  • a is a random number in the array.
  • deckCards[i] is replaced with a;

essentially shuffling 1 card in the array, this happens 51 times?

Your vocabulary is slightly off, and the remarks about i going down to 0 and index being between 1 and 52 are inaccurate.

  • deckCards is a reference to the input array. This function shuffles its elements in place.
  • i is initially the index of the last element of the array.
  • i iterates down to 1.
  • index is a random number between 0 and i, inclusive.
  • a is a random element of the array.
  • deckCards[i] is swapped with a;

essentially shuffling 1 card in the array, this happens 51 times?

However, that is an uninsightful, mechanical description of the code. A better comment would be:

For each element of the array (except the first), starting from the end, pick any previous element (or itself) with uniform probability, and swap them.

This technique is unbiased. For example, consider what happens during the first loop when shuffling a 52-card deck. Every card in the deck is equally likely to take the position of deckCards[51]. With the last element deckCards[51] thus fairly selected, repeat the process — randomly draw from the remaining 51-card deck to fill its last element deckCards[50], and so on for the entire deck.

share|improve this answer
    
Thank you for this, I did a fair amount of research to see what shuffling method would be good. Just an additional question, my comments explaining the fisher-yates method is that correct? I don't like using code I don't understand. –  NiallSzalkai Apr 6 at 12:50
    
Indeed, your comment has some problems. I've added a critique of the comment in my answer. –  200_success Apr 6 at 13:19

There is a small mistake in your code:

Aces are only 11, but not 1. This means the player (or the dealer) could bust with just the two cards they're dealt.
Assuming they only had Aces, then it would be a bust with this code. I would add a check to see if the player is busting, but that one of the cards was dealt as an Ace. Possibly even creating a boolean, PlayerHasAce.

Pseudocode:

If they bust, and PlayerHasAce is TRUE;
    minus 10 from the total;
    are they still busting without additional aces? 
    No? Good! They can continue playing. 

You would just need to make sure the boolean is set only ONCE per Ace though, and not repeated during the same hand.

share|improve this answer
    
your pseudo code isn't complete, if the player has 2 Aces (22) we subtract 10 (12) then gets a ten (22) it should now be (12), they will bust, you need a more complex process for this. –  Malachi May 8 at 16:27
    
@Can if it is only checked once it won't change both aces to have a value of 1, it will only check one of the aces. in this case it should change the second ace to a value of one when the ten card is drawn. it needs to be checked more than once. –  Malachi May 8 at 16:44
1  
Sorry @Malachi, what I meant was that it only does it once per Ace... I'll edit the answer –  Canadian Luke May 8 at 19:30

In addition to creating an object for Deck, an object for a player's hands and the dealer hand will also be useful. In Blackjack a player may take different actions depending on the first two cards dealt, split or double. The player may also take different actions depending on the dealer's up card -- take insurance if it is an ace.

With all that in mind, the basic objects of the game might be:

Deck: holds available cards

DealersHand: holds cards removed from deck and dealt to dealer. One of the first two cards is the up card.

List[Player]: all the players

List[PlayerHands]: each Player has a list of PlayerHands

All hand objects should be able to calculate their score

Finally you may consider a Game object that can deal hands, evaluate the winners of each round, keep score and display the results.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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