5
\$\begingroup\$

I am very new to Java and programming theory and am desperately trying to improve my knowledge. This is the first program I've made without help and really would appreciate some feedback. I know there must be 1,000,000 better ways to do what I did.

Notes:

  • I want to move away from main() programming. How could I achieve this program using methods and OOP?
  • I am aware that the Ace should not just be = 11 and should give the user the chance to dictate its value, but implementing that would not of taught me anything new.
  • The way in which I calculated the dealers & users card value is atrocious, especially because I had to re-iterate it constantly. This is the biggest problem I want to fix.

Images of console to give you an idea of two runs:

enter image description here

enter image description here

Any and all comments are appreciated. I really want to get to grips with basic Java programming concepts before my bad habits are too engraved to break!

import java.util.*;
public class BlackJack {


public static void main(String[] args) {

    /*
     * Scanner && User Variables
     */
    Scanner kb = new Scanner(System.in);
    int usersDecision = 0;

    /*
     * Users && Dealers Value Variables
     */
    int usersValue = 0;
    int dealersValue = 0;

    /*
     * Suit && Rank Arrays
     */
    String[] card = { "2", "3", "4", "5", "6", "7", "8", "9", "10",
            "Jack", "Queen", "King", "Ace"};
    /*
     * Array Lists Users Cards && Dealers Cards
     */
    ArrayList<String> usersCards = new ArrayList<String>();
    ArrayList<String> dealersCards = new ArrayList<String>();


    /*
     * GENERATE DEALERS FIRST CARD
     */
    for (int i = 0; i <= 0; i++) {
        int randomGenNumber = (int) (Math.random()*13);

        dealersCards.add(card[randomGenNumber]);
    }
    /*
     * Print Dealers First Card
     */
    System.out.println("The Dealer Was Dealt: " + dealersCards);


    /*
     * Deal Users Two Cards
     */
    for (int i = 0; i <= 1; i++) {

        int randomGenNumber = (int) (Math.random()*13);

        usersCards.add(card[randomGenNumber]);

    }

    /*
     * Print Users Two Cards
     */
    System.out.println("The User Was Dealt: " + usersCards);

    /*
     * Check For BlackJack
     */
    if(usersCards.contains("Ace")) {
        if(usersCards.contains("King") || usersCards.contains("Queen") || usersCards.contains("Jack") || usersCards.contains("10")){
            System.out.println("You've Got BlackJack! Congratulations, You Win!");
            System.exit(0);
        } else {
            System.out.println("You Did Not Get BlackJack!\n[1] Twist\n[2] Stick");
        }
    } else {
        System.out.println("You Did Not Get BlackJack! :( \n[1] Twist\n[2] Stick");
    }


    /*
     * Take Users Decision
     * Check Users Decision
     * While Loop
     * Switch Statement For Users Decision
     */

    int x = 0;  
    while(x==0) {

        usersDecision = kb.nextInt();

        switch (usersDecision) {
        case 1:
            System.out.println("You've Twisted - Your Cards: " + usersCards);
            System.out.println("You've Twisted - Additional Card Dealt");
            x = 0;

            /*
             * WHILE Twisting = True
             * Generate New Cards
             * Check Value of Cards
             * Bust/Twist/Stick For User
             */
            for (int i = 0; i <= 0; i++) {
                int randomGenNumber = (int) (Math.random()*13);

                usersCards.add(card[randomGenNumber]);
            }

            System.out.println(usersCards + "\n");

            /*
             * Generate Users Card Value
             */
            usersValue = 0;

            for(int i = 0; i < usersCards.size(); i++) {
                if(usersCards.get(i).equals("2")) {
                    usersValue += 2;
                } else if(usersCards.get(i).equals("3")) {
                    usersValue += 3;
                } else if(usersCards.get(i).equals("4")) {
                    usersValue += 4;
                } else if(usersCards.get(i).equals("5")) {
                    usersValue += 5;
                } else if(usersCards.get(i).equals("6")) {
                    usersValue += 6;
                } else if(usersCards.get(i).equals("7")) {
                    usersValue += 7;
                } else if(usersCards.get(i).equals("8")) {
                    usersValue += 8;
                } else if(usersCards.get(i).equals("9")) {
                    usersValue += 9;
                } else if(usersCards.get(i).equals("10")) {
                    usersValue += 10;
                } else if(usersCards.get(i).equals("Jack")) {
                    usersValue += 10;
                } else if(usersCards.get(i).equals("Queen")) {
                    usersValue += 10;
                } else if(usersCards.get(i).equals("King")) {
                    usersValue += 10;
                } else if(usersCards.get(i).equals("Ace")) {
                    usersValue += 11;
                }
            }
            /*
             * Print Users Value
             */
            System.out.println("Users Cards Value: " + usersValue + "");

            if(usersValue != 21 && usersValue <=21){
                System.out.println("You Did Not Get BlackJack!\n[1] Twist\n[2] Stick");

            } else if (usersValue == 21) {
                System.out.println("You Got BlackJack! Congratulations!");
            } else if (usersValue > 21) {
                System.out.println("You've Bust! You Lose!");
                System.exit(0);
            }

            break;

        case 2: 
            System.out.println("You've Stuck - Your Cards: " + usersCards +"\n");
            x = 1;
            /*
             * For Loop
             * Generate Users Card Value
             */
            usersValue = 0;

            for(int i = 0; i <usersCards.size(); i++) {
                if(usersCards.get(i).equals("2")) {
                    usersValue += 2;
                } else if(usersCards.get(i).equals("3")) {
                    usersValue += 3;
                } else if(usersCards.get(i).equals("4")) {
                    usersValue += 4;
                } else if(usersCards.get(i).equals("5")) {
                    usersValue += 5;
                } else if(usersCards.get(i).equals("6")) {
                    usersValue += 6;
                } else if(usersCards.get(i).equals("7")) {
                    usersValue += 7;
                } else if(usersCards.get(i).equals("8")) {
                    usersValue += 8;
                } else if(usersCards.get(i).equals("9")) {
                    usersValue += 9;
                } else if(usersCards.get(i).equals("10")) {
                    usersValue += 10;
                } else if(usersCards.get(i).equals("Jack")) {
                    usersValue += 10;
                } else if(usersCards.get(i).equals("Queen")) {
                    usersValue += 10;
                } else if(usersCards.get(i).equals("King")) {
                    usersValue += 10;
                } else if(usersCards.get(i).equals("Ace")) {
                    usersValue += 11;
                }
            }
            /*
             * Dealers Second Card 
             * &&
             * Check Dealers Value && Take Action
             */
            System.out.println("Dealing Dealers Second Card!");

            for (int i = 0; i <= 0; i++) {
                int randomGenNumber = (int) (Math.random()*13);

                dealersCards.add(card[randomGenNumber]);
            }
            System.out.println(dealersCards + "\n");

            /*
             * For Loop
             * Generate Dealers Card Value
             */
            dealersValue = 0;

            for(int i = 0; i < dealersCards.size(); i++) {
                if(dealersCards.get(i).equals("2")) {
                    dealersValue += 2;
                } else if(dealersCards.get(i).equals("3")) {
                    dealersValue += 3;
                } else if(dealersCards.get(i).equals("4")) {
                    dealersValue += 4;
                } else if(dealersCards.get(i).equals("5")) {
                    dealersValue += 5;
                } else if(dealersCards.get(i).equals("6")) {
                    dealersValue += 6;
                } else if(dealersCards.get(i).equals("7")) {
                    dealersValue += 7;
                } else if(dealersCards.get(i).equals("8")) {
                    dealersValue += 8;
                } else if(dealersCards.get(i).equals("9")) {
                    dealersValue += 9;
                } else if(dealersCards.get(i).equals("10")) {
                    dealersValue += 10;
                } else if(dealersCards.get(i).equals("Jack")) {
                    dealersValue += 10;
                } else if(dealersCards.get(i).equals("Queen")) {
                    dealersValue += 10;
                } else if(dealersCards.get(i).equals("King")) {
                    dealersValue += 10;
                } else if(dealersCards.get(i).equals("Ace")) {
                    dealersValue += 11;
                }
            }
            /*
             * Print Dealers Value
             */
            System.out.println("Dealers Cards Value: " + dealersValue + "");

            /*
             * Take Action On Dealers Value
             */
            int y = 0;
            while(y==0) {

                dealersValue = 0;   
                for(int i = 0; i < dealersCards.size(); i++) {
                    if(dealersCards.get(i).equals("2")) {
                        dealersValue += 2;
                    } else if(dealersCards.get(i).equals("3")) {
                        dealersValue += 3;
                    } else if(dealersCards.get(i).equals("4")) {
                        dealersValue += 4;
                    } else if(dealersCards.get(i).equals("5")) {
                        dealersValue += 5;
                    } else if(dealersCards.get(i).equals("6")) {
                        dealersValue += 6;
                    } else if(dealersCards.get(i).equals("7")) {
                        dealersValue += 7;
                    } else if(dealersCards.get(i).equals("8")) {
                        dealersValue += 8;
                    } else if(dealersCards.get(i).equals("9")) {
                        dealersValue += 9;
                    } else if(dealersCards.get(i).equals("10")) {
                        dealersValue += 10;
                    } else if(dealersCards.get(i).equals("Jack")) {
                        dealersValue += 10;
                    } else if(dealersCards.get(i).equals("Queen")) {
                        dealersValue += 10;
                    } else if(dealersCards.get(i).equals("King")) {
                        dealersValue += 10;
                    } else if(dealersCards.get(i).equals("Ace")) {
                        dealersValue += 11;
                    }
                }
                /*
                 * If Dealers Value:
                 * <=16 == 17 < 17 == 21 > 21
                 * 
                 */
                if(dealersValue <= 16) {
                    int randomGenNumber = (int) (Math.random()*13);
                    dealersCards.add(card[randomGenNumber]);
                    System.out.println("Dealer Has Less Than 17 - Taking Another Card\n");
                    System.out.println("Dealers Cards: " + dealersCards);
                    dealersValue = 0;

                    for(int i = 0; i < dealersCards.size(); i++) {
                        if(dealersCards.get(i).equals("2")) {
                            dealersValue += 2;
                        } else if(dealersCards.get(i).equals("3")) {
                            dealersValue += 3;
                        } else if(dealersCards.get(i).equals("4")) {
                            dealersValue += 4;
                        } else if(dealersCards.get(i).equals("5")) {
                            dealersValue += 5;
                        } else if(dealersCards.get(i).equals("6")) {
                            dealersValue += 6;
                        } else if(dealersCards.get(i).equals("7")) {
                            dealersValue += 7;
                        } else if(dealersCards.get(i).equals("8")) {
                            dealersValue += 8;
                        } else if(dealersCards.get(i).equals("9")) {
                            dealersValue += 9;
                        } else if(dealersCards.get(i).equals("10")) {
                            dealersValue += 10;
                        } else if(dealersCards.get(i).equals("Jack")) {
                            dealersValue += 10;
                        } else if(dealersCards.get(i).equals("Queen")) {
                            dealersValue += 10;
                        } else if(dealersCards.get(i).equals("King")) {
                            dealersValue += 10;
                        } else if(dealersCards.get(i).equals("Ace")) {
                            dealersValue += 11;
                        }

                    }
                    System.out.println("Dealers Cards Value: " + dealersValue + "\n");
                } 

                /*
                 * Checks dealersValue against usersValue
                 * Prints Response
                 */
                if(dealersValue == 17 ) {
                    System.out.println("Dealer Has 17 - Dealer Sticks\n");
                    y = 1;

                    if(usersValue < 17) {
                        System.out.println("You Have: " + usersValue + " You Lost");
                    } else if(usersValue == dealersValue) {
                        System.out.println("You Have: " + usersValue + " You Drew");
                    } else {
                        System.out.println("You Have: " + usersValue + " You Won!");
                    }
                }

                if(dealersValue > 17 && dealersValue < 21) {
                    System.out.println("Dealer Has: " + dealersValue + " Dealer Sticks\n" );
                    y = 1;

                    if(usersValue < 18) {
                        System.out.println("You Have: " + usersValue + " You Lost");
                    } else if(usersValue == dealersValue) {
                        System.out.println("You Have: " + usersValue + " You Drew");
                    } else {
                        System.out.println("You Have: " + usersValue + " You Won!");
                    }
                }

                if(dealersValue == 21) {
                    System.out.println("Dealer Has BlackJack\n");
                    y = 1;
                    if(usersValue == dealersValue) {
                        System.out.println("You Have: " + usersValue + " You Drew");
                    }
                }

                if(dealersValue > 21) {
                    System.out.println("Dealer Has Busted - You Win!");
                    y = 1;
                }

            }
            break;
        default:
            System.out.println("Not A Valid Selection");
        }

    };
  }
}
\$\endgroup\$
6
\$\begingroup\$
  1. I'd use enums for the cards instead of Strings. It would make the code safer (since the compiler could check typos which is not possible with Strings). Related Effective Java, 2nd Edition chapters:

    • Item 50: Avoid strings where other types are more appropriate
    • Item 30: Use enums instead of int constants

    The enum values could contain the value of the card. It would improve cohesion and help you to get rid of the big if-else if structures, so it'd replace conditional with polymorphism. Reference:

    • Refactoring: Improving the Design of Existing Code by Martin Fowler: Replacing the Conditional Logic on Price Code with Polymorphism
  2. Furthermore, I'd create a Hand class to store the cards which a user or a dealer has, and it could have at least the following methods:

    • void addCard(Card card)
    • hasBlackJack()
    • int getCardsValue()

    Code Complete 2nd Edition has a good chapter about this (Working Classes).

  3. The type of the usersCards reference could be simply List<...>:

    List<...> usersCards = new ArrayList<...>();
    

    Reference: Effective Java, 2nd edition, Item 52: Refer to objects by their interfaces

  4. 1 and 2 are magic numbers (in [1] Twist\n[2] Stick and in the switch-case). It should be a named constant. It would improve readability.

\$\endgroup\$
6
\$\begingroup\$

You seem to be repeating sections like this.

for (int i = 0; i <= 0; i++) {
    int randomGenNumber = (int) (Math.random()*13);

    dealersCards.add(card[randomGenNumber]);
}

First, it does not make sense to use a for loop if the number of loops are just one. On the other hand, I also notice that you have

for (int i = 0; i <= 1; i++) {
    int randomGenNumber = (int) (Math.random()*13);
    usersCards.add(card[randomGenNumber]);
}

So why not refactor both into a separate method?

static final int MaxCard = 13;
void dealCards(ArrayList<String> cards, int num) {
  for (int i = 0; i < num; i++) {
    cards.add(card[(int) (Math.random()*MaxCard)]);
  }
}

And you can use it thus

dealCards(dealersCards, 1); // for the first type
dealCards(usersCards, 2); // for the second type

Also note that it is better to take out numbers such as 13 and declare them to be a constant with a good variable name that denotes their purpose. It helps you to understand your code at a later time.

--

usersValue = 0;
for(int i = 0; i < usersCards.size(); i++) {
    if(usersCards.get(i).equals("2")) {
        usersValue += 2;
    } else if(usersCards.get(i).equals("3")) {
        usersValue += 3;
    } else if(usersCards.get(i).equals("4")) {
        usersValue += 4;
    } else if(usersCards.get(i).equals("5")) {
        usersValue += 5;
    } else if(usersCards.get(i).equals("6")) {
        usersValue += 6;
    } else if(usersCards.get(i).equals("7")) {
        usersValue += 7;
    } else if(usersCards.get(i).equals("8")) {
        usersValue += 8;
    } else if(usersCards.get(i).equals("9")) {
        usersValue += 9;
    } else if(usersCards.get(i).equals("10")) {
        usersValue += 10;
    } else if(usersCards.get(i).equals("Jack")) {
        usersValue += 10;
    } else if(usersCards.get(i).equals("Queen")) {
        usersValue += 10;
    } else if(usersCards.get(i).equals("King")) {
        usersValue += 10;
    } else if(usersCards.get(i).equals("Ace")) {
        usersValue += 11;
    }
}

These sections seem to be repeating again and again. You could make them into a method like above, and call it instead of repeating them.

class NoParse extends Exception {}
...
String[] tenValcards = {"Jack", "Queen", "King"};
int parseCard(String card) throws NoParse {
   char c = card.charAt(0);
   int i = Character.digit(c,10);
   switch(i) {
      case -1: {
         for (String c : tenValueCards)
             if c.equals(card) return 10;
         break;
      }
      case 1: {
         i = Character.digit(card.charAt(1),10);
         if (i == 0) return 10;
         break;
      }
      case 0: break;
      default: return i;
   }
   // handle the parse error.
   throw new NoParse(card);
}   

Using it,

usersValue += parseCard(usersCards.get(i));
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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