HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMajor

"Guess a number" game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
gamenumberguess

Problem

I began the journey to be a self-taught programmer about a month ago and my biggest fear about teaching myself is developing bad habits and not realizing it.

I'm looking for criticism on this "Guess a Number" game. It's my first real "project" that I've invested serious time into so I feel like this is my first opportunity to be critiqued.

```
package guessANumber;

import java.util.Random;

import javax.swing.JOptionPane;

/**
* @author Mike Medina
*/

/**
* The "Guess a Number" game
/
public class Guess {

static boolean playAgain;
static int max; // The objective will be between 1 and this number
static int objective; // Users are trying to guess this number
static int userGuess;

public Guess() {}

/**
* Runs the game while playAgain == true
*/
public static void main(String[] args) {

do {
setMax();
setObjective();
userGuess();
playAgain();
} while (playAgain == true);

}

/**
* Asks the user to set the max value they will guess between and validates it
*/
public static void setMax() {

boolean valid = false;

// Asks user for "max"
while(!valid) {
try {
max = Integer.parseInt(JOptionPane.showInputDialog("You will try to guess a number between 1 and \"max\". Set max: "));
valid = true;
}
catch (NumberFormatException e) {}
}

// Ensures user input for max is greater than 1 and fits in an int
while (max Integer.MAX_VALUE) {
valid = false;
while (!valid) {
try {
max = Integer.parseInt(JOptionPane.showInputDialog("Max must be between 1 and " + Integer.MAX_VALUE + ": "));
valid = true;
}
catch(NumberFormatException e) {}
}
}

}

/**
* Sets the objective between 1 and "max"
*/
public static void setObject

Solution

On top of the comments already given :

Keep things simple :

  • while (playAgain == true) can be written while (playAgain)



  • if (A) variable=false; else variable=true; can be written variable = !A;



  • Do not repeat yourself : if you deal a lot with user input and checking values, make it a function you can reuse.



  • You do not need static members. Local variable can do the trick. The idea is to keep things in the smallest possible scope so that it's easier to understand what changes what.



Here's my code taking into account my comments :

import java.util.Random;
import javax.swing.JOptionPane;

/**
 * The "Guess a Number" game
 * */
public class Guess {
    public Guess() {}

    /**
     * Runs the game while playAgain == true
     */ 
    public static void main(String[] args) {
        do {
            int max = getMax();
            int obj = getObjective();
            userGuess(obj, max);
        } while (playAgain());
    }

    private static int getIntFromUser(String text, int min, int max)
    {
        while (true)
        {
            try
            {
                int val = Integer.parseInt(JOptionPane.showInputDialog(text));
                if (min  objective)
            JOptionPane.showMessageDialog(null, "Too high!");
        else
            JOptionPane.showMessageDialog(null, "You win!");
    }

    /**
     * Asks the user if they want to play again or quit
     */
    public static boolean playAgain() {
        return (getIntFromUser("Enter 0 to quit or 1 to play again", 0, 1) == 1);
    }
}

Code Snippets

import java.util.Random;
import javax.swing.JOptionPane;

/**
 * The "Guess a Number" game
 * */
public class Guess {
    public Guess() {}

    /**
     * Runs the game while playAgain == true
     */ 
    public static void main(String[] args) {
        do {
            int max = getMax();
            int obj = getObjective();
            userGuess(obj, max);
        } while (playAgain());
    }

    private static int getIntFromUser(String text, int min, int max)
    {
        while (true)
        {
            try
            {
                int val = Integer.parseInt(JOptionPane.showInputDialog(text));
                if (min <= val && val <= max)
                    return val;
            }
            catch (NumberFormatException e) { /* As pointed out by ChrisW, it might be worth having something here */}
        }
    }

    /**
     * Asks the user for the max value
     */
    public static void getMax() {
        return getIntFromUser("You will try to guess a number between 1 and \"max\". Set max: ", 1, Integer.MAX_VALUE);
    }

    /**
     * Gets the objective between 1 and "max"
     */
    public static int getObjective() {
        // Please read Simon André Forsberg's comment about this.
        Random rand = new Random();
        return (max == 1) ?
            1 :
            rand.nextInt(max - 1) + 1;
    }

    /**
     * Takes in the user's guess, validates it, and tells them when they win
     */
    public static void userGuess(int obj, int max) {
        int userGuess;
        do {
            userGuess = getIntFromUser("Guess a number between 1 and " + max + ": ", 1, max);
            userWinLose(userGuess, obj);
        } while (userGuess != obj);
    }

    /**
     * Tells the user whether their guess was high, low, or correct
     */
    public static void userWinLose(int guess, int objective) {
        if (guess < objective)
            JOptionPane.showMessageDialog(null, "Too low!");
        else if (guess > objective)
            JOptionPane.showMessageDialog(null, "Too high!");
        else
            JOptionPane.showMessageDialog(null, "You win!");
    }

    /**
     * Asks the user if they want to play again or quit
     */
    public static boolean playAgain() {
        return (getIntFromUser("Enter 0 to quit or 1 to play again", 0, 1) == 1);
    }
}

Context

StackExchange Code Review Q#39459, answer score: 26

Revisions (0)

No revisions yet.