10
\$\begingroup\$

I'm learning how to code in Java and I decided to re-write a program made in C++ into Java. All feedback is welcome.

Here is the original prompt:

This program generates a pseudo-random number between 1 and 100, inclusive. The user will then be presented with opportunities to guess that number. After each guess, the program indicates whether the guess was too high, too low, or correct. The user will bet on each number. Initially, the program will give the user $1000. Each round,the program prompts the user for a bet. The program then prompts for guesses until the user is correct, or has made 6 wrong guesses. Once the round has ended, either by a correct guess or by using up the 6 guesses, the program displays the current status and prompts the user to see if he/she wants to play again. This will continue until the user indicates that they do not want to play again or until the player runs out of money.

package high_low_game;
import java.util.*;
/**
 *
 * @author mjoseph2017
 */
public class High_low_game {



public static void main(String[] args) 
{
    int money = 1000;
    int min = 1;
    int max = 100;
    int gamesWon = 0;
    int gamesPlayed = 1;
    boolean playAgain = true;
    double percentage = (gamesWon / gamesPlayed) * 100;



    while (playAgain == true)
    {
        printHeading(money, min, max);
        playGame(money, max, min, percentage);
    }

    double winnings;
    winnings = playGame (money, max, min, percentage);

    if (money <= winnings) // compares the money won to amount at the begginning at the game
    {
        gamesWon++; // games won increses if you gain more money
        gamesPlayed++;
    }

    else 
    {
        gamesPlayed++;
    }        



    // prompt user to play again
}

public static int printHeading (int money, int min, int max)
{
    System.out.println("=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-="); 
    System.out.println("Welcome to the High-Low Betting Game"); 
    System.out.println("You have $" + money + " to begin the game"); 
    System.out.println("Valid guess are numbers between " + min + " and " + max); 
    System.out.println("=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=");

    return money;

}

public static int playGame (int money, int max, int min, double percentage)
{
    /*Scanner input = new Scanner(System.in);
    int bet = input.nextInt();*/
    int guess = getGuess();

    int bet = getBet(money);

    int numDrawn = drawNum(max, min);


    for (int num = 1; num <= 6; num++) // determines if guess is high, low, or     correct
{
    System.out.print("Guess " + num + ": " + guess);
            System.out.println();

    if (guess > numDrawn)
    {
                System.out.print("Too high...");
    }

    if (guess < numDrawn)
    {
                System.out.print("Too low...");
    }

    if (guess == numDrawn)
    {
                System.out.print("Correct!");
            }

    }
    calcNewMoney(money, bet, guess);
    playAgain(percentage);

    return money;
}

/**
 *
 * @param money
 * @return     
 */
public static int getBet(int money)
{
    System.out.println("Please enter a bet");
    Scanner input = new Scanner(System.in);
    int bet = input.nextInt();

    if (bet < 1 || bet > 100)
    {
        System.out.println("That is not a valid bet");
        getBet(money);
    }

    return bet;
}

public static int getGuess ()
{
    Scanner input = new Scanner(System.in);
    System.out.println("Please enter a guess");
    int guess = input.nextInt();
    return guess;
}

public static int drawNum (int max, int min)
{
    Random rand = new Random();
    int randNum = rand.nextInt((max - min) +1)+ min;

    return randNum;
}

public static int calcNewMoney (int money, int bet, int guess)
{
    int earnings = bet / guess;

    if (guess <= 6)
    {
        money = money + earnings;
    }

    else
    {
        money = money - bet;
    }

    System.out.println("You have won $" + earnings);
    System.out.println("You have $" + money);

    return money;
}

public static boolean playAgain (double percentage)
{
    boolean playAgain = true;
    Scanner input = new Scanner(System.in);

    System.out.println("You have won " + percentage + "% of the games played");

    System.out.println("Would you like to play again? y or n");

    String again = input.nextLine();

    if (again.equalsIgnoreCase("y"))
    {
        playAgain = true;
    }

    else if (again.equalsIgnoreCase("n"))
    {
        playAgain = false;
    }

    return playAgain;
  }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ 1.) format your code with auto-formatting, 2.) fix the error because you had a semi-colon aka ; at the end of your while loop which just makes it not work as you would expect. \$\endgroup\$ – Zhuinden Nov 12 '14 at 14:25
  • 3
    \$\begingroup\$ @konijn because of at least double percentage = (gamesWon / gamesPlayed) * 100; \$\endgroup\$ – Heslacher Nov 12 '14 at 15:36
  • 2
    \$\begingroup\$ You really should fix the ; at the end of that while. \$\endgroup\$ – Zhuinden Nov 12 '14 at 15:45
  • 1
    \$\begingroup\$ A good old division by zero. \$\endgroup\$ – Morwenn Nov 12 '14 at 15:46
  • 1
    \$\begingroup\$ I appreciate your vigilance @Heslacher ! \$\endgroup\$ – konijn Nov 12 '14 at 16:03
10
\$\begingroup\$

In the first place, you have this code:

if (money <= winnings) // compares the money won to amount at the beginning at the game
{
    gamesWon++; // games won increases if you gain more money
    gamesPlayed++;
}

else 
{
    gamesPlayed++;
}

This is redundant. You should do this instead, which also shows what happens better:

gamesPlayed++; // increment gamesPlayed

if (money <= winnings) // increment gamesWon if winnings is <= money
{
    gamesWon++; // increment gamesWon
}

Additionally, your code formatting is awful. You should really format your code so all the levels of indentation are indented the same amount; this will really increase readability.

\$\endgroup\$
10
\$\begingroup\$

BUG and unexpected results

  • Your code does can't escape this loop, unless you are hitting STRG + C

    while (playAgain == true)
    {
        printHeading(money, min, max);
        playGame(money, max, min, percentage);
    }  
    

    because playAgain is never changed. But let us assume this loop wouldn't stay like it is, the user would be forced to play the game again, if he wants to or not.

    winnings = playGame (money, max, min, percentage);  
    
  • From the assignment

    After each guess, the program indicates whether the guess was too high, too low, or correct. The user will bet on each number. Initially, the program will give the user $1000. Each round,the program prompts the user for a bet.

    Your code just don't ask after a incorrect guess for the next number. It just compares 6 times the given number to the guessed one.

    Pst, I will tell you a secret: the numbers don't change , they stay the same.

  • The playAgain() method will evaluate any input which isn't "n" to true.

SRP

At least inside the getBet() method you are vialoating the Single responsibility principle

public static int getBet(int money)
{
    System.out.println("Please enter a bet");
    Scanner input = new Scanner(System.in);
    int bet = input.nextInt();

    if (bet < 1 || bet > 100)
    {
        System.out.println("That is not a valid bet");
        getBet(money);
    }

    return bet;
}

this method is just doing to much. It is printing to the console, reading from System.in and validating the read input and by entering an invalid input, this is done recursively.

General

  • a condition like (playAgain == true) can be expressed by just (playAgain) and if you need to check if it is false, you should use the Not operator ! like (!playAgain)
  • assigning a int to a double variable is bad practice.
  • comments should describe why something is done. What is done should be described by the code itself, by using meaningful class, method, and varaibles names.
  • dead code should be deleted.
  • if you only need java.util.Random and java.util.Scanner you shouldn't import java.util.*
  • you have waaayyyy to many new lines and your indention is totally crap.
  • the preffered way for placing opening braces { is on the same line like

    if (condition) {
    
    }
    
  • classes shouldn't be named using snake_case casing.

  • a if statmenet like

    boolean b;
    if (condition) {
        b = true;
    }
    else if (!condition) {
        b = false;
    }  
    

    can be expressed like

    boolean b = condition;
    
  • the input parameter money in the getBet() method isn't used at all.

  • variable and class names should not be shortened as this would remove readability
  • a Random should be created once and reused

Now, let us get this bugs out of the way.

But let us do this in the right way. We will use only static main() method and only to start the game.

First we add some class level variables which will hold the lowest and highest number which we make final as they won't change for a game. We will use constructor chaining, that means we are calling another constructor of the class form a constructor.

So our base will look like

public class HighLowGame {

    private final int minNumber;
    private final int maxNumber;
    private final int minBet;
    private final int maxBet;

    public HighLowGame() {
        this(1, 100, 1, 100);
    }
    public HighLowGame(int minNumber, int maxNumber) {
        this(minNumber, maxNumber, 1, 100);
    }
    public HighLowGame(int minNumber, int maxNumber, int minBet, int maxBet) {
        this.minNumber = minNumber;
        this.maxNumber = maxNumber;
        this.minBet = minBet;
        this.maxBet = maxBet;
    }

    public void play() {
        int money = initialMoney;
        boolean playAgain = true;

        while (playAgain) {
            money = play(money);
            playAgain = (money != 0) && shouldContinue();
        }
        writeOutput("Thanks for playing. Bye.");
    }

    public static void main(String[] args) {

        HighLowGame game = new HighLowGame();
        game.play();
    }

    private boolean shouldContinue() {
        return readInput("Would you like to play again? y or n").equalsIgnoreCase("y");
    }

    private String readInput(String message) {

        writeOutput(message);

        Scanner input = new Scanner(System.in);
        return input.nextLine();
    }

    private void writeOutput(String message){
        System.out.println(message);
    }
}

now we need to add code for play() but before we will change the drawNum() method to

private final Random random = new Random();
public int drawNumber() {
    return random.nextInt((maxNumber - minNumber) + 1) + minNumber;
}  

and the getBet() method to

private final String bettingMessage = "Please enter a bet";
private final String bettingInvalidMessage = "That is not a valid bet";

public int getBet(int money) {
    int bet = readIntInput(bettingMessage);
    while (!isBetValid(bet, money)) {
        writeOutput(bettingInvalidMessage);
        bet = readIntInput(bettingMessage);
    }
    return bet;
}

private boolean isBetValid(int bet, int money) {
    return (bet >= minBet && bet <= maxBet && bet <= money);
}

you need to change the getGuess() method accordingly.

Here is the entry method for the class.

public void play() {
    int money = initialMoney;
    int playedGames = 0;
    boolean playAgain = true;

    while (playAgain) {
        playedGames += 1;
        money = play(money);
        writeOutput("You have played " + playedGames + " and you have $ " + money + " left");
        playAgain = shouldContinue();
    }
    writeOutput("Thanks for playing. Bye.");
}

and the overloaded play() method

public int play(int money) {

    int playedRounds = 1;
    int currentBet = getBet(money);
    int numberToGuess = drawNumber();

    while (playedRounds <= maxRounds) {
        int guessedNumber = getGuess();

        writeOutput("Guess " + playedRounds + ": " + guessedNumber);

        if (guessedNumber == numberToGuess) {
            writeOutput("Correct!");
            break;
        }

        if (guessedNumber > numberToGuess) {
            writeOutput("Too high...");
        } else {
            writeOutput("Too low...");
        }
        playedRounds += 1;
    }

    writeOutput("The number to guess had been: " + numberToGuess)
    return money - currentBet;
}

You need to add some exception handling for the case that the user doesn't input numbers when he is supposed to do.

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

You asked just for improvements, so there's one: change the printHeading function type to void since you don't need any return.

public static int printHeading(int money, int min, int max) //change int to void
{
    System.out.println("=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=");
    System.out.println("Welcome to the High-Low Betting Game");
    System.out.println("You have $" + money + " to begin the game");
    System.out.println("Valid guess are numbers between " + min + " and " + max);
    System.out.println("=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=");

    return money; // remove this line

}

It will work better this way:

public static void printHeading(int money, int min, int max){
    System.out.println("=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=");
    System.out.println("Welcome to the High-Low Betting Game");
    System.out.println("You have $" + money + " to begin the game");
    System.out.println("Valid guess are numbers between " + min + " and " + max);
    System.out.println("=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=");

}

Here's another:

Move the definition of double winnings to start of main method, and use

winnings = playGame(money, max, min, percentage);

only inside of the while loop instead of playGame().

\$\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.