Guess a number

[Dyler]

Hey there,

I'm actually learning programming (with Java) - just for the case someone is blaming me for my very bad Java-Skills.
I'm presenting the following very simple code, because I want to get some advices for improving my programming skills ... and may it will help someone in the future.

Is the code good to read or are there any big mistakes in formatting?
Did I split the code correctly? Or is there something that I should add to a separate class?
Anything else that seems bad?

Game.java:

import java.util.Scanner;

public class Game {
	private Scanner sc = new Scanner(System.in);
	private RandomNumber search = new RandomNumber();
	private int input;
	private int counter = 0;
	
	private void tellInstructions() {
		System.out.println("The computer generates a random positive number between 0 and 100.");
		System.out.println("Your goal is to guess the right number in as less steps as possible.");
	}
	
	private void newGame() {
		search.setNumber(100);
		counter = 1;
	}
	
	private int getGuess() {
		System.out.print("What's your guess? (-1 to exit):");
		return sc.nextInt();
	}
	
	private boolean checkGuess(int n) {
		if (n == search.getNumber())
			return true;
		else
		return false;
	}
	
	private void checkDifference(int n) {
		if (n > search.getNumber())
			System.out.println("The searched number is less than your input!");
		if (n < search.getNumber())
			System.out.println("The searched number is bigger than your input!");
	}
	
	private void victory(){
		System.out.println("Congratulations, you won with just " + counter + " steps!");
		counter = 0;
	}
	
	public void start() {
		tellInstructions();
		while (true) {
			if (counter==0) newGame();
			input = getGuess();
			if (input==-1) break;
			if (checkGuess(input)) victory();
			else {
				checkDifference(input);
				counter++;
			}
		}
		System.out.println("Hope you enjoyed the game. Have a nice day!");
	}
	public static void main (String argvc[]) {
		Game game = new Game();
		game.start();
	}
}

RandomNumber.java:

public class RandomNumber {
	private int number;
	
	public RandomNumber() {
		setNumber(0);
	}

	public int getNumber() {
		return number;
	}

	public void setNumber(int n) {
		this.number = (int) (Math.random() * n);
	}
}

Greets,
Dyler

EDIT: Just not sure if the Thread "What do you need help with? Version 5" would have been a better choice for this...

[20k]

[QUOTE=Dyler;35484423]

	private void checkDifference(int n) {

		        if (n > search.getNumber())
			        System.out.println("The searched number is less than your input!");

		        if (n < search.getNumber())
			        System.out.println("The searched number is bigger than your input!");

	}

Greets,
Dyler

EDIT: Just not sure if the Thread "What do you need help with? Version 5" would have been a better choice for this...[/QUOTE]

Personally i like to space out code slightly more than you do. However, the entire point of code spacing is to make your code readable, and seeing as it is you who primarily will read your code, space it as best you see fit

[Map in a box]

private void checkDifference(int n) {

		        if (n > search.getNumber())
			        System.out.println("The searched number is less than your input!");

		        if (n < search.getNumber())
			        System.out.println("The searched number is bigger than your input!");

	}

You are getting the number twice there. Store it in a temporary variable then do the comparisons

[Dyler]

[QUOTE=Icedshot;35484729]Personally i like to space out code slightly more than you do.[/QUOTE]
I'm too lazy for pressing tab more than once :-)

[QUOTE=Map in a box;35485004]
You are getting the number twice there. Store it in a temporary variable then do the comparisons[/QUOTE]
What would the advantage be? I thought it would be better to read without storing it temporary.

[Zyx]

If you ask me, then this is bad:
[cpp]
private Scanner sc = new Scanner(System.in);
private RandomNumber search = new RandomNumber();
private int input;
private int counter = 0;
[/cpp]

Instead do
[cpp]
private Scanner sc;
private RandomNumber search;
private int input;
private int counter;

public Game() {
sc = new Scanner(System.in);
search = new RandomNumber();
counter = 0;
}
[/cpp]

input should also not be a global variable as it's only used in a single method.

To clarify: I think bracketless if statements are bad.
[cpp]
if(whatever())
{
//Much better
}
[/cpp]

[20k]

[QUOTE=Zyx;35487158]
Bracketless if statements are also bad.
[cpp]
if(whatever())
{
//Much better
}
[/cpp][/QUOTE]

I am curious, enlighten me as to why bracketless if statements are resolutely bad, despite being in a situation where the brackets serve no purpose in clarification

[neos300]

[QUOTE=Icedshot;35487282]I am curious, enlighten me as to why bracketless if statements are resolutely bad, despite being in a situation where the brackets serve no purpose in clarification[/QUOTE]

If you have to add more lines later it's annoying to add the brackets.
Much easier to just have them there in the first place.

[Asheron]

	private boolean checkGuess(int n) {
		if (n == search.getNumber())
			return true;
		else
		return false;
	}

Whenever you see something like that, you can easily change it in to the following...

	private boolean checkGuess(int n){
			return n == search.getNumber();
	}

It's not a big deal but it's more elegant.

[Zyx]

[QUOTE=Icedshot;35487282]I am curious, enlighten me as to why bracketless if statements are resolutely bad, despite being in a situation where the brackets serve no purpose in clarification[/QUOTE]

This mainly:
[QUOTE=neos300;35489743]If you have to add more lines later it's annoying to add the brackets.
Much easier to just have them there in the first place.[/QUOTE]

Also I just think it looks a lot nicer and adds to readability. Especially if you also add else statements and nested if else blocks.
And because Netbeans auto generates the brackets for me :v:

[Monster Bait]

The computer generates a random positive number between 0 and 100.
Your goal is to guess the right number in as less steps as possible.
What's your guess? (-1 to exit):50
The searched number is bigger than your input!
What's your guess? (-1 to exit):75
The searched number is less than your input!
What's your guess? (-1 to exit):62
The searched number is less than your input!
What's your guess? (-1 to exit):56
Congratulations, you won with just 4 steps!
What's your guess? (-1 to exit):

Win for me on first go :D

Criticism (i'm also a novice Java Programmer and would like counter criticisms):

the start() method could be completely removed and the contents placed within the Game's constructor

private Game() {
// start() method code goes here.
}

You should always use a constructor for initialization of starting variables and any methods which are associated with any starting functions of that class.

You should probably never write
while(true)
whatever your break condition is you should place it in the while loop brackets.
As you can set variables as well as compare them in statements you could even write it like:
while ((input = getGuess())!=-1)
this will set input to whatever the user inputs for their guess and only continue if the input is not equal to -1 ... it also removes a chunk of code from within the while loop if you do it like this.

You could encapsulate your entire random number class within your game class as a nested class as it is not used by any other class, this would also allow you to use private methods in that class.
(loop up class nesting if your are interested)

this statement is pointless:

if (n == search.getNumber())
return true;
else
return false;

you may as well simply write:

return (n == search.getNumber());

Also some rudimentary error handling could be implemented (users are always stupid and putting in letters ) :)

That's about everything, your doing a good job and you understand a great deal :)
I even learnt how to use a scanner which I had never used before :P

This is my second post ever on FP and I don't know how to put the code in special boxes so please tell
:D
ty!

[Zyx]

[QUOTE=Monster Bait;35504277]This is my second post ever on FP and I don't know how to put the code in special boxes so please tell
ty![/QUOTE]

If you want to post your code do it like this:
[noparse][cpp]
Code goes here
[/cpp][/noparse]

[Dyler]

[QUOTE=Monster Bait;35504277]
the start() method could be completely removed and the contents placed within the Game's constructor
[/QUOTE]
If I would add a menu, than I wouldn't be able to start the game a second time, would I?

[QUOTE=Monster Bait;35504277]
return (n == search.getNumber());
[/Quote]
You're genius. I didn't even think about doing it this way.

[QUOTE=Monster Bait;35504277]
You could encapsulate your entire random number class within your game class as a nested class as it is not used by any other class, this would also allow you to use private methods in that class.
(loop up class nesting if your are interested)
[/Quote]
Wouldn't that be a violation of the basic idea of 'private'? I'm always using private methods as 'help'-methods fot the class itself and 'public'-methods for the communication to the outside. (Well, I'm actually not sure if the grammar is correct ...)

[QUOTE=Monster Bait;35504277]
You should probably never write
while(true)
[/quote]
I'm trying to avoid this, but in this case did it seem legit.

[Overv]

I understand that you used a program as simple as this to demonstrate this approach of abstracting everything, but you're really going overboard with it here, so I'm going to discuss it anyway.

This is most apparent example of pointless functions:

[cpp]private boolean checkGuess(int n) {
if (n == search.getNumber())
return true;
else
return false;
}[/cpp]

There is literally no reason to have this around. If [i]n == search.getNumber()[/i] is too long for you, you're doing something wrong.

Another problem is your RandomNumber class. You've probably been taught that encapsulation is a good thing, which it is, but it's really not necessary in this case. I doubt you're going to change the way random numbers are generated any time soon, so this class is 100% pointless. Besides that, the class offers no advantage over just using a variable to store the random number.

Here is how I would implement this application:

[cpp]import java.util.Scanner;

public class Game
{
private const Scanner scan = new Scanner( System.in );

public static void main( String argvc[] )
{
System.out.println( "The computer generates a random positive number between 0 and 100." );
System.out.println( "Your goal is to guess the right number in as few attempts as possible." );

int target = (int)( Math.random() * 100 );
int guesses = 1;

while ( true )
{
System.out.print( "What's your guess? (-1 to exit):" );
int input = scan.nextInt();

if ( input < target )
System.out.println( "Too low!" );
else if ( input > target )
System.out.println( "Too high!" );
else if ( input == -1 )
break;
else if ( input == target )
{
System.out.println( "Congratulations, you got it right after " + guesses + " guesses!" );
target = (int)( Math.random() * 100 );
guesses = 1;
}

guesses++;
}

System.out.println( "Hope you enjoyed the game. Have a nice day!" );
}
}[/cpp]

Note that I do not have a lot of experience with Java.

[DrLuke]

For the love of god, don't do this:

[cpp]int foo() {
bar()
}[/cpp]

But do it like this:

[cpp]int foo()
{
bar()
}[/cpp]

The first one is incredibly annoying to read

[Spoco]

[QUOTE=DrLuke;35509792]For the love of god, don't do this:

[cpp]int foo() {
bar()
}[/cpp]

But do it like this:

[cpp]int foo()
{
bar()
}[/cpp]

The first one is incredibly annoying to read[/QUOTE]

[URL]http://www.oracle.com/technetwork/java/codeconv-138413.html[/URL]

Follow the Java coding conventions. He did it right.

[Zyx]

[QUOTE=Spoco;35510125][URL]http://www.oracle.com/technetwork/java/codeconv-138413.html[/URL]

Follow the Java coding conventions. He did it right.[/QUOTE]

It all comes down to personal preference and what you think is most readable.

As an example most people clearly love bracketless if statements while I prefer to avoid that.
Same with this. I like the 4 line version, while others prefer the 3 line options.

[Darwin226]

[QUOTE=DrLuke;35509792]For the love of god, don't do this:

int foo() { bar()}

But do it like this:

int foo() { bar()}

The first one is incredibly annoying to read[/QUOTE]

It's best to get used to both. You never know where you might be working some day and what conventions you'll be forced to follow.

[Monster Bait]

[QUOTE=Dyler;35508112]If I would add a menu, than I wouldn't be able to start the game a second time, would I?

You're genius. I didn't even think about doing it this way.

Wouldn't that be a violation of the basic idea of 'private'? I'm always using private methods as 'help'-methods fot the class itself and 'public'-methods for the communication to the outside. (Well, I'm actually not sure if the grammar is correct ...)

I'm trying to avoid this, but in this case did it seem legit.[/QUOTE]

I ment to say you could put the start method in the constructor but you could have it separate .. you could have code in your main method to create a new instance of the object for a new game, or have it in a different method you call when you start the game, it's perfectly good like that :)

after trying to rewrite the code I found that the while(true) loop is the most obvious and to me the most logical way of doing it! I remember reading somewhere, maybe in a book, that you should never say while(true){ //blabla }
anyway, any more q's about java welcome

[Jetsurf]

[QUOTE=Darwin226;35515771]It's best to get used to both. You never know where you might be working some day and what conventions you'll be forced to follow.[/QUOTE]

Usually, when you are working with a language, you use the languages conventions. I don't mind either one, but I always use what the language convention says to do...

3 lined version for C/++/Java
4 for C#

etc, etc...