Please Evaluate My First App!

[mtorbin]

Hey all,

I'm still very much in learning mode here, so please help me out. The following code works fine, but I'm wondering if my if statements aren't a bit hoakey? This is the first application I wrote without looking at the book (too much). Please let me know what I could do to improve this (please bare in mind I'm still very new so keep it simple).

Thanks,

- MT

// RandomNumber.java
// Guess a random number between 1 and 100 within 20 guesses

import java.util.Random;

public class RandomNumber {
    private int maxNumber;
    private int totalGuesses;
    private int x;
    
    public RandomNumber() {
        maxNumber = 100;
        Random rand = new Random();
        x = rand.nextInt(maxNumber+1);
      
    }
    
    public void takeAGuess(int guessedNumber, int count) {
        if((guessedNumber < x) && (guessedNumber != x)) {
            System.out.println("Oooh... so close, you'll need to guess higher than that.\n");
        }
        
         if((guessedNumber > x) && (guessedNumber != x)) {
            System.out.println("Yeah, about that... um, can we try a little lower?\n");
         } 
                
         if(guessedNumber == x) {
             totalGuesses = 20 - count;
             System.out.println("Dude, you guessed it and in only " + totalGuesses + " tries!");
         }
    }
    
    public int getRN() {
        return x;
    }
}

// RunRNGame.java
// Runs RandomNumberGame.java program

import java.util.Scanner;

public class RNGame {
    public static void main(String args[]) {
        RandomNumber newGame = new RandomNumber();
        int guessCount;
        int userGuess;
        int targetNumber;
        
        System.out.println("Welcome to the Random Number Game!");
        targetNumber = newGame.getRN();
        guessCount = 20;
        Scanner input = new Scanner(System.in);
        userGuess = 0;
        
        while ((guessCount > 0) && (userGuess != targetNumber)) {
            System.out.println("You currently have " + guessCount + " chances left.");
            System.out.print("What is your guess? ");
            userGuess = input.nextInt();
            newGame.takeAGuess(userGuess, guessCount);
            --guessCount;
        }
        
    }
}

 

[stefanwagner]

maxNumber is only used locally, so it shouldn't be a membervariable.
The same is true for totalGuesses.

if (gn < x && gn != x) ...

Can you tell a value for x, such that gn < x and gn == x?

RandomNumber newGame

is a curious naming strategie.

totalGuesses = 20 - count;
System.out.println("Dude, you guessed it and in only " + totalGuesses + " tries!");

Well - that's an assumption, which the method can't guarantee.
It is dependend on the caller to send the right values, so it shouldn't tell such things, or ensure, to tell the truth.
The magic number 20 is stored at two places, which might easily lead to an error (changing one value, but not the other one).

And more general: In which way is your RandomNumber reusable?
System.out.-messages are often not very useful.

You could improve your design by thinking about:
Which values are likely to be changed? (maxGuesses, ...)
How could the logic be used with a totally different userinterface (a GUI (swing), a text2speach-interface, a webservice).

Wouldn't it be nice to use the commandline-arguments to specify an upperbound for the random-value, and a maximum-guess-count value?

seeking a job as java-programmer in Berlin:

http://home.arcor.de/hirnstrom/bewerbung

 

[mtorbin]

Thanks, I appreciate the comments. Actually, I made some changes and I'm having trouble seeing them work. The problem is that once the user sets the upper bound it doesn't refelect in the random number:

// RandomNumber.java
// Guess a random number between 1 and 100 within 20 guesses

import java.util.Random;

public class RandomNumber {
    private int maxNumber;
    private int totalGuesses;
    private int x;
    private int warningCount;
    
    public RandomNumber() {
        maxNumber = 100;
        warningCount = 0;
        Random rand = new Random();
        x = rand.nextInt(maxNumber+1);
      
    }
    
    public void takeAGuess(int guessedNumber, int count, String name) {
        if(warningCount > 2) {
            System.out.println("OK, are you just being stupid on purpose??  Look, let me go over this for you.\nNumbers OVER " + maxNumber + " are not part of the game, OK??");
        }
    
        if((guessedNumber > maxNumber) && (warningCount < 3)) {
            System.out.println("Are you insane??  Can you not read?? I said BETWEEN 1 and " + maxNumber + "!!  ");
            ++warningCount;
        }
    
        if((guessedNumber < x) && (guessedNumber != x)) {
            System.out.println("Oooh... so close, you'll need to guess higher than that.\n");
        }
        
         if((guessedNumber > x) && (guessedNumber != x)) {
            System.out.println("Yeah, about that... um, can we try a little lower?\n");
         } 
                
         if(guessedNumber == x) {
             totalGuesses = 20 - count;
             System.out.println(name + ", you guessed it and in only " + totalGuesses + " tries!");
         }
    }
    
    public int getRN() {
        return x;
    }
    
    public void setMaxNumber(int mn) {
        maxNumber = mn;
        Random userRand = new Random();
        x = userRand.nextInt(maxNumber+1);
    }
    
    public int getMaxNumber() {
        return maxNumber;
    }
}

// RunRNGame.java
// Runs RandomNumberGame.java program

import java.util.Scanner;

public class RNGame {
    public static void main(String args[]) {
        RandomNumber newGame = new RandomNumber();
        int guessCount;
        int userGuess;
        int targetNumber;
        String userName;
        int max;
        
        System.out.println("Welcome to the Random Number Game!");        
        targetNumber = newGame.getRN();
        guessCount = 20;
        Scanner input = new Scanner(System.in);
        userGuess = 0;
        
        System.out.print("Please tell me your name: ");
        userName = input.nextLine();
        
        System.out.print("Please tell me the maximum number that you wish to use: ");
        max = input.nextInt();
        newGame.setMaxNumber(max);
        
        System.out.println("\nThank you, " + userName + ".  Here's the rules of the game:");
        System.out.println("You have twenty chances to guess my number between 1 and " + newGame.getMaxNumber() + ".");
        System.out.println("While I won't tell you what the number is, I will give you hints.");
        System.out.println("Pretty simple, right?  OK then, let's get started!");
       
        while ((guessCount > 0) && (userGuess != targetNumber)) {
            System.out.println("You currently have " + guessCount + " chances left.");
            System.out.println(guessCount + "\n" + targetNumber);
            System.out.print("What is your guess? ");
            userGuess = input.nextInt();
            System.out.println();
            newGame.takeAGuess(userGuess, guessCount, userName);
            --guessCount;
        }
        
    }
}

I realize this is a bit hoakey, but how can I get the method setMaxNumber to affect the maxNumber variable in the Random Number generator?

- MT

 

[stefanwagner]

a) I tested your code, and it accepted an upper bound of 5000.
Perhaps you modify your new version and execute an old one?

b) You still use:
if (gn < x && gn != x) ...
Why?

seeking a job as java-programmer in Berlin:

http://home.arcor.de/hirnstrom/bewerbung

 

[mtorbin]

Stefan,

a) Yes, it will accept it, but if you try something like an upper bound of 5 and guess it in three guesses it keeps going.

b) I don't understand. You'll have to explain to me in a more basic way. Why is this not OK?

- MT

 

[Diancecht]

Because if a < b then always a != b, it's just redundant.

Cheers,
Dian

 

[stefanwagner]

You first get the target number...

targetNumber = newGame.getRN();
//
newGame.setMaxNumber(max);

... and update it later, without getting the new target-number.

takeAGuess could be defined to return a boolean, if the number is guessed:

         if(guessedNumber == x) {
             totalGuesses = 20 - count;
             System.out.println(name + ", you guessed it and in only " + totalGuesses + " tries!");
             return true;
         }
         return false;

This would avoid having the number stored at two places which allways will lead to inconsistencies.

SPOS: Single point of storage - or what's the acronym from the pragmatic programmers?

seeking a job as java-programmer in Berlin:

http://home.arcor.de/hirnstrom/bewerbung