Hi,

I've seen a program called guessing game in a book. There are basically 3 players in the game. The program generates a random number and the players guess for that number. I wrote my own code for it and need your advice on how to improve the Object Oriented concept in the program. Please tell me the design flaws in my program. Please find the program below:

NOTE: I've named the classes as they were in the book itself, but the code is different.

GameLauncher.class:- This is the main class

package com.XXX;

public class GameLauncher {

	/**
	 * @param args
	 */
	public static void main(String[] args) {

			GuessGame game = new GuessGame();
			game.startGame();
	}

}

Player.class: - In this class the player guesses the number and it is compared to the targetnumber.

package com.XXX;

public class Player {
	
	static int number;
	
	public boolean guess(int targetNumber)
	{
		number= (int) (Math.random()*10);
		System.out.println("the number guessed is"+number);
		if(number==targetNumber)
		{
			return true;
		}
		else
			return false;
	}

}

GuessGame.class : - This class generates the targetnumber.

package com.vamsi;

public class GuessGame {
 
	Player player1;
	Player player2;
	Player player3;
	
public void startGame()
{
	player1 = new Player();
	player2 = new Player();
	player3 = new Player();
	
	int targetNumber =(int) (Math.random() *10);
	System.out.println("the number to be guessed is "+targetNumber);
	while(true)
	{
		
		boolean result1 = true;
		boolean result2 = true;
		boolean result3 = true;
		result1 = player1.guess(targetNumber);
		result2 = player2.guess(targetNumber);
		result3 = player3.guess(targetNumber);
		
		if(result1==true)
		{
	     		System.out.println("the correct number is guessed by player1. The number is "+Player.number);
				break;
		}
		else if(result2==true)
		{
				System.out.println("the correct number is guessed by player2. The number is "+Player.number);
				break;
		}
		else if(result3==true)
		{
			System.out.println("the correct number is guessed by player3. The number is "+Player.number);
			break;
		}
		/*else
		{
			System.out.println("The number guessed is"+Player.number);
		}*/
	}
}
				
		
		
}

Thanks,
NewCoder :)

There's nothing wrong with that as an OO design. Well done.
The main thing you could improve are the player1/player2/player3 variables. What happens if you want four players? It would be a lot better to have an array of Players (and an array of boolean results) - this will eliminate a lot of repeated code (repeated code is a bad thing) and allow you to pass the number of players as a parameter to a constructor for the GuessGame class.
One bug: Player's number variable should NOT be static - that means all the Players share the same value for number, which they do not. It would also be clearer if you called it "guess" rather than the not-very-descriptive "number"

Edited 5 Years Ago by JamesCherrill: static bug

Hi,

Not much to do to improve the OO design in your code. Looks pretty good indeed. I addition to what JamesCherrill said, I would probably just add a constructor to the Player class:

public Player() {
    this.number = -1;
}

in case you want to extend it later.

..I would probably just add a constructor to the Player class ... in case you want to extend it later.

Not sure I understand this suggestion. The assignment number = -1; seems unnecessary, and without it all you have is the default constructor that the compiler will create for you anyway. Or did I miss something?

Since you did ask for feedback...

if(number==targetNumber)
		{
			return true;
		}
		else
			return false;
	}

is a long way of saying

return (number==targetNumber);

There is one major design flaw here. Player shall just make a guess. The decision on the guess correctness does not belong to Player; it belongs to GuessGame itself.

There is one major design flaw here. Player shall just make a guess. The decision on the guess correctness does not belong to Player; it belongs to GuessGame itself.

I agree that the decision would be better in GuessGame rather than Player. However, I wouldn't describe that as a "major design flaw" for someone just starting out. One step at a time...

This article has been dead for over six months. Start a new discussion instead.