I made a Three Monte Card game using Java
Problem:
This is the original "ball and cups" game where you try to find out which cup has the ball under it. You may play with three cups and a ball, three cards (two jacks and an ace), or three doors and a car. Basically, randomly select a cup to hide the "ball". Let the player guess which cup hides the ball. The player wins if they guess correctly.
Problem taken from:
https://programmingbydoing.com/a/three-card-monte.html
My code:
/*
* Code by CLint
*/
import java.util.Random;
import java.util.Scanner;
public class ThreeCardMonte {
public static void main(String[] args) {
Random random = new Random();
Scanner input = new Scanner(System.in);
int userInput;
int randomN = random.nextInt(3)+1;
System.out.println("You slide up to Fast Eddie's card table and plop down your cash.\n" +
"He glances at you out of the corner of his eye and starts shuffling.\n" +
"He lays down three cards.\n");
System.out.println("Which one is the ace?\n" +
"\t##\t##\t##\n" +
"\t##\t##\t##\n" +
"\t1\t2\t3");
System.out.print("\n> ");
userInput = input.nextInt();
if (userInput == randomN) {
if (randomN == 1) {
System.out.println("\nYou nailed it! Fast Eddie reluctantly hands over your winnings, scowling.\n" +
"\tAA\t##\t##\n" +
"\tAA\t##\t##\n" +
"\t1\t2\t3");
} else if (randomN == 2) {
System.out.println("\nYou nailed it! Fast Eddie reluctantly hands over your winnings, scowling.\n" +
"\t##\tAA\t##\n" +
"\t##\tAA\t##\n" +
"\t1\t2\t3");
} else if (randomN == 3) {
System.out.println("\nYou nailed it! Fast Eddie reluctantly hands over your winnings, scowling.\n" +
"\t##\t##\tAA\n" +
"\t##\t##\tAA\n" +
"\t1\t2\t3");
}
}
if (userInput != randomN) {
if (randomN == 1) {
System.out.println("\nHa! Fast Eddie wins again! The ace was card number 1.\n" +
"\tAA\t##\t##\n" +
"\tAA\t##\t##\n" +
"\t1\t2\t3");
} else if (randomN == 2) {
System.out.println("\nHa! Fast Eddie wins again! The ace was card number 2.\n" +
"\t##\tAA\t##\n" +
"\t##\tAA\t##\n" +
"\t1\t2\t3");
} else if (randomN == 3) {
System.out.println("\nHa! Fast Eddie wins again! The ace was card number 3.\n" +
"\t##\t##\tAA\n" +
"\t##\t##\tAA\n" +
"\t1\t2\t3");
}
}
}
}
The output:
PS: I am open for corrections to make my code for efficient and clean.
-
\$\begingroup\$ there is not much left for efficency since this program flow is linear. \$\endgroup\$Martin Frank– Martin Frank2020年07月27日 04:49:46 +00:00Commented Jul 27, 2020 at 4:49
2 Answers 2
single responsibility
your class does all things together
- reading user input
- printing dialogs
- handle game logic
by this you violate the openClosed principle. If you want to change your code (eg. play a one of four game) you have to make a lot of changes. If you want to add handling for InputExceptions
(see Scanner.nextInt()
you would find the proper place. Make Classes for each responsibility!
minor issues
- magic numbers:
(3)+1
- hardcoded Strings (put them in a language file) - that also helps to separate concerns between dialogs and logic
- redundancy (use a formatter for the text from the language files)
Constants
A couple of constants would make your code easier to adapt. So, for example a constant for the number of cards would allow the potential to adapt the program to 2/4 card play.
Variable naming
To make the program easier to follow, I favour names that indicate what a variable represents, rather than where it comes from. So, rather than randomN
, consider acePosition
. Rather than userInput
, consider guessedPosition
.
Duplicated logic
When you're doing things that are identical / very similar it's a good indication that there's scope to extract into a function / class. There are two main duplications that you've got, when printing out each of the winning/losing statements with the card positions. These also share printing the end card positions. Consider extracting a method that can output the card positions, with the ace in a given position.
If/Else
When you've got two if conditions that are exclusive, use if/else, rather than if(userInput == randomN)
and if(userInput != randomN)
Putting it together
You might end up with something more like this:
public class ThreeCardMonte {
final static int UNKNOWN_POSITION = 0;
final static int NUMBER_OF_CARDS = 3;
public static String formatCards(int acePosition) {
var cardValue = IntStream.range(0, NUMBER_OF_CARDS)
.mapToObj(i -> i + 1 == acePosition ? "\tAA" : "\t##")
.collect(Collectors.joining()) + "\n";
var columnLabels = IntStream.range(0, NUMBER_OF_CARDS)
.mapToObj(i->"\t"+(i+1))
.collect(Collectors.joining()) + "\n";
return cardValue + cardValue + columnLabels;
}
public static void main(String[] args) {
Random random = new Random();
Scanner input = new Scanner(System.in);
int acePosition = random.nextInt(NUMBER_OF_CARDS) + 1;
System.out.println("You slide up to Fast Eddie's card table and plop down your cash.\n" +
"He glances at you out of the corner of his eye and starts shuffling.\n" +
"He lays down three cards.\n");
System.out.println("Which one is the ace?\n" + formatCards(UNKNOWN_POSITION));
System.out.print("> ");
int guessedPosition = input.nextInt();
if (guessedPosition == acePosition) {
System.out.println("\nYou nailed it! Fast Eddie reluctantly hands over your winnings, scowling.\n" +
formatCards(acePosition));
} else {
System.out.format("\nHa! Fast Eddie wins again! The ace was card number %d.\n" +
formatCards(acePosition), acePosition);
}
}
}