4
\$\begingroup\$

I know some things are a bit over the top (such as the hashmap use, but it was needed to use some sort of collection class) - but are there anything else that sticks out as a sore thumb in my code? I'll add comments to all my code eventually.

I was also told to make it as object oriented as I could. Have I achieved it?

Main Class

public class Main {
    public static void main (String[] args) throws InterruptedException{
    Player deck = new Player();     
    deck.displayPerson();
    }
}

Player Class

import java.util.Scanner;



public class Player extends Dealer  {
static int playerTotal;

@SuppressWarnings("resource")
public void displayPerson() throws InterruptedException {


    Thread.sleep(2000);
    System.out.println("\nShuffling cards... Will deal in 2 seconds.");
    Thread.sleep(1000);
    System.out.println("\n-------------------------\n");
    Thread.sleep(1000);


    System.out.println("You drew the cards: ");
    Thread.sleep(800);
    Deck.getCardfromDeck("Player");
    Thread.sleep(800);
    System.out.println("\n    & ");
    Thread.sleep(800);
    Deck.getCardfromDeck("Player");

    Thread.sleep(1000);
    System.out.println("\nYour total is: " + playerTotal + "!");
    Thread.sleep(1000);
    System.out.println("\nThe dealer drew the card");
    Thread.sleep(500);
    Deck.getCardfromDeck("Dealer");
    Thread.sleep(800);
    System.out.println("And a secret card he wont show yet");
    Thread.sleep(1200);
    System.out.println(" ______");
    System.out.println("|?     |");
    System.out.println("|   ?  |");
    System.out.println("| ?  ? |");
    System.out.println("|      |");
    System.out.println("|_____?|");
    Thread.sleep(1000);
    // asking them if they want to stay or hit again
    System.out.println("Would you like to 'hit' or 'stay'?");
    Scanner scan = new Scanner(System.in);
    String hitorstay = scan.nextLine();
    Thread.sleep(800);

    // if they want to hit, do this
    if (hitorstay.equalsIgnoreCase("hit")) {

        System.out.println("You drew an ");
        Deck.getCardfromDeck("Player");
        System.out.println("\n Your total is now " + playerTotal);
        Thread.sleep(800);
        if (playerTotal > 21) {
            Thread.sleep(800);
            System.out
                    .println("You busted, total is over 21, you lost, and you got "
                            + playerTotal);
            System.exit(1);
        }
        Thread.sleep(800);
        System.out.println("Would you like to 'hit' or 'stay?'");
        hitorstay = scan.nextLine();
    }
    else if (hitorstay.equals("stay")) {
        System.out.println("Okay, dealers turn");
        Thread.sleep(800);
        Dealer.displayDealer();
    }
else{
     Scanner errorScan = new Scanner(System.in);
        System.out.println("You typed wrong. Would you like to play again? Y/N");
    String playagain = errorScan.nextLine();
        if(playagain.equalsIgnoreCase("Y")){
            System.out.println("Would you like to 'hit' or 'stay'?");
            hitorstay = scan.nextLine();}
        else
            System.out.println("Good bye. Have a nice day!");
            System.exit(0);
    }
}
}

Dealer Class

    public class Dealer  {

        static int dealerTotal;

        public static void displayDealer() throws InterruptedException {



            System.out.println("The secretcard that the dealer had was: ");
            Deck.getCardfromDeck("Dealer");
            System.out.println("\nTheir total is now: " + dealerTotal);
            Thread.sleep(800);


            // The dealer should hit when his score is <17
            while (dealerTotal < 17) {

                Thread.sleep(800);
                System.out.println("The dealer gets a new card: ");
                Deck.getCardfromDeck("Dealer");
                System.out.println("\n Their total is now " + dealerTotal);
            }
            if (dealerTotal > 21) {
                Thread.sleep(800);
                System.out
                .println("They busted, total is over 21, and they lost, you won! Their total was "
                        + dealerTotal);
                System.exit(1);
            }
            else if (dealerTotal >= 17) {
                Thread.sleep(800);
                System.out.println("Okay, dealer stopped at " + dealerTotal);
                WhoWon.displayWhoWon();
            }
        }
    }

WhoWon Class

    public class WhoWon extends Player{
        public static void displayWhoWon(){

            if(playerTotal > dealerTotal){
                System.out.println("The Player won! The players score was " + playerTotal + ", and the dealers score was " + dealerTotal);
            }
            else if(playerTotal < dealerTotal){
                System.out.println("The Dealer won! The players score was " + playerTotal + ", and the dealers score was " + dealerTotal);
            }
        }
    }

Deck Class

import java.util.HashMap;
import java.util.Map;
import java.util.Random;

public class Deck extends Player {

//Get the card from the Deck
    public static void getCardfromDeck(String name) {

        newValor(name);
    }

    //Parameter 'name' for whom the card should go to.
    public static void newValor(String name) {
         Random rng = new Random();
         int currentcard = rng.nextInt(13);
         currentcard++;

            Map<Integer, String> nameMap = new HashMap<Integer, String>();

            nameMap.put(1, "A");
            nameMap.put(10, "T");
            nameMap.put(11, "J");
            nameMap.put(12, "Q");
            nameMap.put(13, "K");


            String getValor;
            if (currentcard >= 10 || currentcard == 1) {
                getValor = nameMap.get(currentcard);
                if (name.equals("Player")){
                playerTotal = playerTotal + 10;}
                else if (name.equals("Dealer")){
                dealerTotal = dealerTotal + 10;
                }
                System.out.println(" ______");
                System.out.println("|"+getValor+"     |");
                newSuit();
                System.out.println("|_____"+getValor+"|");
            }
            else{

                if(name.equals("Player"))
                playerTotal = currentcard + playerTotal;
                else if(name.equals("Dealer"))
                dealerTotal = currentcard + dealerTotal;


                System.out.println(" ______");
                System.out.println("|"+currentcard+"     |");
                newSuit();
                System.out.println("|_____"+currentcard+"|");
            }
    }


    public static void newSuit() {

        Random rng = new Random();
        int generateSuit = rng.nextInt(4);
        generateSuit++;

        Map<Integer, String> suitMap = new HashMap<Integer, String>();

        suitMap.put(1, "Spades");
        suitMap.put(2, "Diamonds");
        suitMap.put(3, "Clubs");
        suitMap.put(4, "Heart");

        if (generateSuit == 1) {

            System.out.println("|  /\\  |");
            System.out.println("| (__) |");
            System.out.println("|  /\\  |");    
        }
        else if (generateSuit == 2) {

            System.out.println("|  /\\  |");
            System.out.println("|  \\/  |");
            System.out.println("|      |");

        } else if (generateSuit == 3) {

            System.out.println("|  &   |");
            System.out.println("| &|&  |");
            System.out.println("|  |   |");

        } else if (generateSuit == 4) {

            System.out.println("| (\\/) |");
            System.out.println("|  \\/  |");    
            System.out.println("|      |");
        }
    }
}
\$\endgroup\$

2 Answers 2

7
\$\begingroup\$

OOP

The code isn't really object oriented, but procedural.

First of all, you don't have any objects. You have classes, but you never create instances of them.

Also, all your methods are static. Sometimes, static functions make sense, but if you have too many, it's a sign of bad design.

Your class inheritance also doesn't make all that much sense; you can think of extends as is-a to check if your inheritance is correct. For example, is a Deck a kind of Player? Or is a Player a kind of Dealer? They are not, so those extends shouldn't exist.

Also, there really isn't such a thing as a WhoWon, this shouldn't be a class, but a function inside for example a Game class.

OOP - a better approach

When creating classes, try to think of the things like they exist in the real world. What do you have at a blackjack table? You have cards, you have hands, you have player, you have dealer, maybe you also have a table and a game. So you can create classes for all these things.

Then, think about what those things have and what they do, and create appropriate fields and methods. For example, a Card has a suit and a value, so add those fields.

A Hand has Cards, it has a hand-value, and it can take more cards.

Here is some code to give you an idea:

public class Card {

    private Rank rank;
    private Suit suit;

    public Card(Rank rank, Suit suit) {
        this.rank = rank;
        this.suit = suit;
    }

    public Rank getRank() {
        return rank;
    }

    public Suit getSuit() {
        return suit;
    }

    @Override
    public Suit toString() {
        // make asci art here
    }

    public enum Rank {

        TWO(2),
        THREE(3),
        FOUR(4),
        FIVE(5),
        SIX(6),
        SEVEN(7),
        EIGHT(8),
        NINE(9),
        TEN(10),
        JACK(11),
        QUEEN(12),
        KING(13),
        ACE(14);

        private int value;

        private Rank(final int value) {
            this.value = value;
        }

        public int getValue() {
            return value;
        }
    }

    public enum Suit {

        SPADES, HEARTS, DIAMONS, CLUBS
    }
}

public class Deck {
    List<Card> cards;

    public Deck() {
        cards = new ArrayList<>();
        Card.Rank[] ranks = Card.Rank.values();
        Card.Suit[] suits = Card.Suit.values();
        for (Card.Rank rank : ranks) {
            for (Card.Suit suit : suits) {
                cards.add(new Card(rank, suit)); // create a new instance of the Card class
            }
        }

    }

    public void shuffle() {
        // TODO shuffle
    }

    public List<Card> getCards(int amount) {
        // return amount of cards
    }
}

And a game might look like this (very abstract, just to give you an idea):

public class Game {

    private Dealer dealer; // dealer has a Deck
    private List<Player> player; // player has a hand, which has a list of cards, and methods such as getTotalValue, etc

    public void playOneRound() {
        while (!quit) {
            dealer.deal(player);
            applyUserInput(getUserInput());
            dealer.play();
            checkWin(dealer, player);
        }
        outputResults(dealer, player);
    }
}

You might want to check out this post about OOP in Poker and some of the questions in Playing Cards, and also read about Object-Oriented Programming Concepts in general.

\$\endgroup\$
10
  • 1
    \$\begingroup\$ @JohnPulple I think it would be easier start from scratch. I would first create the classes you think you need, then add method signatures, then create and implement the game loop, and then implement all the other methods. For the last step, you can copy-paste some of your old code (like the printing of the suits). You might also want to sketch out your class design before starting to write any code (check out UML for this; you don't need to draw 100% correct and complete UML diagrams, but it does help to get a good overview). \$\endgroup\$
    – tim
    Oct 15, 2014 at 15:21
  • 1
    \$\begingroup\$ @JohnPulple Yeah, linking it together isn't always easy. The main concern should be that coupling of the objects is as low as possible. Code gets very hard to maintain when a lot of objects reference each other. Again, a good first approach is to think about how it is in real-life: a Dealer has a Deck, and a dealer also deals cards to a player (Dealer:deal(Player player, int amountOfCards)). A Player has a Hand, to which cards can be added (Player:addCard(List<Card> cards), this can be used by the Dealer. \$\endgroup\$
    – tim
    Oct 15, 2014 at 16:25
  • 1
    \$\begingroup\$ [cont.] The addCard method would add the cards to the Hand field of the player (a Dealer of course also has a hand. So Player and Dealer could both extend a base class, such as HandHolder (cannot think of a better name); or you could say that a Dealer is a certain kind of Player, and thus let Dealer extend Player. I also updated my answer to give you a better idea. \$\endgroup\$
    – tim
    Oct 15, 2014 at 16:26
  • 1
    \$\begingroup\$ @JohnPulple i'm glad to help :) I'm not sure I quite understand your question, but the best approach now is probably to just try around yourself - get a feel for it - until you have working code that makes sense to you, and then post a follow-up question. \$\endgroup\$
    – tim
    Oct 15, 2014 at 17:02
  • 1
    \$\begingroup\$ @JohnPulple override toString of all classes. Then you can easily get output and see current state of objects. You can also use the implemented toString method of the objects an object stores. And you should use toString for your cards as well. You can do the actual printing in the playOneRound loop I posted above, for example, after dealing, call System.out.println(dealer.toString()), where the toString of dealer calls the toString of Hand (and maybe adds some info), which calls the toString of Card. \$\endgroup\$
    – tim
    Oct 15, 2014 at 17:14
5
\$\begingroup\$

I already posted something about the general concept and OOP, but I also have some comments on the concrete code:

displayWhoWon

You have

        if(playerTotal > dealerTotal){
            // report
        } else if(playerTotal < dealerTotal){
            // report
        }

But what if the scores are equal? You should add an else block reporting a draw.

getCardfromDeck

The getCardfromDeck method doesn't do anything, except call a different method. Such wrapper methods are quite useless, I would just delete it.

Random

Instead of

     int currentcard = rng.nextInt(13);
     currentcard++;

you can write

    int currentcard = rng.nextInt(13) + 1;

Naming

  • use camelCase for variable names to make them easier to read (eg hitorstay should be hitOrStay, playagain should be playAgain, etc.).
  • Main:main(): why is the Player variable named deck? player would seem more logical.

Usability

  • it's ok to not implement all rules and possibilities of blackjack, but I would add that ace can be 1 or 11 (you use 10, which is definitely wrong).
  • if I mistype hit/stay, and then say yes, i would like to play again, the game exists.
  • I wouldn't use sleep at all, it makes it seem like the game lags (but if you do, store the time in a field, so it's easy to disable this).
  • I would print the cards of a hand next to each other, not below each other.
  • I like the asci art :)

Formatting

  • your indentation is off sometimes (for example in newValor or displayPerson).
  • you are also a bit inconsistent with the use of spaces
  • you should use curly braces even for one-line statements (for readability and to avoid bugs).

If you use an IDE, it can fix these things for you.

Misc

  • you should try to limit where you are printing. Right now, every method in every class prints. Try to replace some of those prints with the returning of a string (or ideally an object in most cases), and then print that in your main game class.
  • remove unused variables (suitMap for example is never used).
  • don't hard-code magic numbers, but store them in a field instead. That way, you can give them a name, and it will also be clearer which magic numbers are the same on purpose, and which are the same by accident.
  • your fields should be private.
\$\endgroup\$

Your Answer

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

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