Problem
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
Aceshould 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:
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] Twistn[2] Stick");
}
} else {
System.out.println("You Did Not Get BlackJack! :( n[1] Twistn[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] Twistn[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 Cardn");
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 Sticksn");
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 Sticksn" );
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 BlackJackn");
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");
}
};
}
}
Solution
-
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
-
Furthermore, I’d create a
Handclass 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).
-
The type of the
usersCardsreference could be simplyList<...>:List<...> usersCards = new ArrayList<...>();Reference: Effective Java, 2nd edition, Item 52: Refer to objects by their interfaces
-
1and2are magic numbers (in[1] Twistn[2] Stickand in the switch-case). It should be a named constant. It would improve readability.
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));