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.

\$\endgroup\$
1
  • \$\begingroup\$ there is not much left for efficency since this program flow is linear. \$\endgroup\$ 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)
\$\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);
        }
    }
}
\$\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.