4
\$\begingroup\$

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:

Code Output

PS: I am open for corrections to make my code for efficient and clean.

asked Jul 26, 2020 at 12:13
\$\endgroup\$
1
  • \$\begingroup\$ there is not much left for efficency since this program flow is linear. \$\endgroup\$ Commented Jul 27, 2020 at 4:49

2 Answers 2

4
\$\begingroup\$

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)
answered Jul 27, 2020 at 5:13
\$\endgroup\$
1
\$\begingroup\$

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);
 }
 }
}
answered Jul 27, 2020 at 11:29
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.