patternjavaMajor
"Guess a number" game
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
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 :
Here's my code taking into account my comments :
Keep things simple :
while (playAgain == true)can be writtenwhile (playAgain)
if (A) variable=false; else variable=true;can be writtenvariable = !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.