Guessing game - am I using the concept of objects in Java properly?

Question

I'm new to Java and find it hard to grasp the concept of objects. I wrote a simple guessing game to practice the concept of OO programming but I am not sure if I am doing it correctly.

The objective of the game is to guess a number from 0-9 with 3 players and the first player who guesses it correctly will win the game.

Player

public class Player {

    private String playerName;
    private int number;

    public int getNumber() {
        return number;
    }

    public String getPlayerName() {
        return playerName;
    }

    public void setPlayerName(String playerName) {
        this.playerName = playerName;
    }

    public void guessNumber(){
        number = Integer.parseInt(JOptionPane.showInputDialog(playerName 
            + "'s Turn\nGuess the Number"));
        JOptionPane.showMessageDialog(null, playerName+ "'s GUESS is " + number);
    }
}

GuessGame

public class GuessGame {

    private int numberToGuess;
    private Player p1;
    private Player p2;
    private Player p3;

    public void startGame(){
        numberToGuess = (int) (Math.random()*10);
        p1 = new Player();
        p2 = new Player();
        p3 = new Player();

        p1.setPlayerName(JOptionPane.showInputDialog("Enter Player 1 Name: "));
        p2.setPlayerName(JOptionPane.showInputDialog("Enter Player 2 Name: "));
        p3.setPlayerName(JOptionPane.showInputDialog("Enter Player 3 Name: "));

        int flagWinner = 0;
        while(0==flagWinner){
            p1.guessNumber();
            if (p1.getNumber()==numberToGuess){
                flagWinner = 1;
                break;
            }
            JOptionPane.showMessageDialog(null, p1.getPlayerName()
                + "'s Guess is Wrong!");
            p2.guessNumber();
            if (p2.getNumber()==numberToGuess){
                flagWinner = 2;
                break;
            }
            JOptionPane.showMessageDialog(null, p2.getPlayerName()
                + "'s Guess is Wrong!");
            p3.guessNumber();
            if (p3.getNumber()==numberToGuess){
                flagWinner = 3;
                break;
            }
            JOptionPane.showMessageDialog(null, p3.getPlayerName()
                + "'s Guess is Wrong!");
        }
        if (1 == flagWinner){
            JOptionPane.showMessageDialog(null,p1.getPlayerName()+ " Wins!");
        } else if (2 == flagWinner){
            JOptionPane.showMessageDialog(null,p2.getPlayerName()+ " Wins!");
        } else JOptionPane.showMessageDialog(null,p3.getPlayerName()+ " Wins!");
    }
}

And I have a gameLauncher where I have the main() method which creates the game.

Am I doing it correctly especially in the Player class where I created a method guessNumber() instead of setter setNumber(int number)?

Am I violating any object-oriented concepts in my code?

How can I improve my code?

Peter Taylor · Accepted Answer · 2011-04-11 09:00:20Z

To start with, spot the inconsistency. For the name you create it in your main loop and then use a setter; for the number you have a method in the Player object to acquire the guess. The latter is more in line with the message-passing philosophy of OO. (Many people seem to think that using getters and setters is being OO. They're often - not always - a sign of non-OO thinking). However, it doesn't take it as far as it could. The number guessed isn't a property of the player - it's a property of the guess.

Guessing a number isn't the same thing as displaying it - either rename the method or, probably better - a method should do one thing - move that out.

Next up, the flagWinner is a big red flag. Its sole purpose is to identify one of the objects p1, p2, p3, or none of the above. What's the OO way to do that? Use the object to identify itself.

I'd be inclined to factor out the IO as an object too, to be a bit less procedural. So without going into a full over-patterned architecture we have

public class IO {
    public void output(String msg) {
        JOptionPane.showMessageDialog(null, msg);
    }

    public String input(String prompt) {
        return JOptionPane.showInputDialog(prompt)
    }

    public int inputInt(String prompt) {
        return Integer.parseInt(input(prompt));
    }
}

public class Player {
    private String playerName;

    private Player(String name) {
        playerName = name;
    }

    public static Player createPlayer(IO io, String prompt) {
        String name = io.input("Enter " + prompt + " Name: ");
        return new Player(name);
    }

    public String getPlayerName() {
        return playerName;
    }

    public int guessNumber(IO io) {
        return io.inputInt(playerName +"'s Turn\nGuess the Number"));
    }
}

public class GuessGame {

    private final IO io;
    private final int numPlayers;
    private int numberToGuess;
    private List<Player> players = new LinkedList<Player>();

    public GuessGame(IO io, int numPlayers) {
        this.io = io;
        this.numPlayers = numPlayers;
    }

    public void init() {
        numberToGuess = (int) (Math.random()*10);
        for (int i = 0; i < numPlayers; i++) {
            players.add(Player.createPlayer(io, "Player " + (i+1)));
        }
    }

    public void runGame(){
        init();

        Player winner = null;
        Iterator<Player> it = players.iterator();
        while (winner == null) {
            if (!it.hasNext()) {
                it = players.iterator();
            }

            Player turnPlayer = it.next();
            String playerName = turnPlayer.getPlayerName();

            int guess = turnPlayer.guessNumber(io);
            io.output(playerName+ "'s GUESS is " + number);

            if (guess == numberToGuess) {
                winner = turnPlayer;
            }
            else {
                io.output(playerName + "'s Guess is Wrong!");
            }
        }

        io.output(winner.getPlayerName()+ " Wins!");
    }
}

Your IO-class has no attributes, so all methods could be made static, and every instance removed/replaced with IO. And a lot of minor coding error can be found: IO.output should be void, inputInt return an Int, createPlayer has to be static, the second it = p.iterator should be it=p.next (); String playerName with capital S and so on. — user unknown, Apr 11, 2011 at 6:37
@user, most of those points are correct, thanks. IO is deliberately not made static because I was refactoring the OP's code with an eye to making it easy to make more OO, and that class is en-route to becoming an interface. And the second it=p.iterator() is correct: it resets the iterator to the start to allow looping through the players more than once. — Peter Taylor, Apr 11, 2011 at 9:02
IO should not be static, it is good the way it is. This design makes it easy to write different versions (e.g. a console input version, or a version for JUnit tests), and to switch to Dependency Injection (e.g. Spring or Guice) later. static is not an OO concept (e.g. doesn't play nice with inheritance) and should be avoided if possible, except for "constants" and for functions that will never ever change (say Math.sin). — Landei, Apr 11, 2011 at 9:14
p.next, I see now. :) My fault. @Landei (Landei? Hi!): I have to think about the static question, and hope I will not forget it. — user unknown, Apr 11, 2011 at 9:27
@deian, if you just want to link to the answer, there's a "share" link between the answer itself and the comments. It gives you a short URL which includes your user ID, and people following it are counted for the "Announcer" family of badges. If you want to quote parts of the answer verbatim in your own answer, you can use the "edit" link to copy the original Markdown, and then if you add > before each line it will be processed into a blockquote. You should also link back to the original answer. See blog.stackoverflow.com/2009/06/attribution-required — Peter Taylor, Jul 22, 2016 at 7:09

palacsint · Accepted Answer · 2012-11-11 10:14:40Z

The layout felt very C++ and very connected to "functions" instead of objects. Though you seem to got a nice idea of how it's work and do a lot better at objects than most people do.

I would do it like this (dirty version, should be improved, but it was a quick redesign):

public class Player {
    private String playerName;
    private int number;

    public Player(String name){
      this.playerName = name;
    }

    public int guessNumber() {
        return Integer.parseInt(JOptionPane.showInputDialog(playerName
            +"'s Turn\nGuess the Number"));
        JOptionPane.showMessageDialog(null, playerName+ "'s GUESS is " + number);
    }

    public String getPlayerName() {
        return playerName;
    }
}

public class GuessGame {

    private int numberToGuess;
    private Player p1;
    private Player p2;
    private Player p3;

    //Would probably made amount of players dynamic as well.
    public GuessGame(String player1, String player2, String player3){
        numberToGuess = (int) (Math.random()*10);
        p1 = new Player(player1);
        p2 = new Player(player2);
        p3 = new Player(player3);
    }

    //Would probably split this up in submethods too.
    //The flagging is quite ugly too but I don't got time to edit it.
    public void startGame(){
        int flagWinner = 0;
        while(0==flagWinner){
            if (p1.guessNumber()==numberToGuess){
                flagWinner = 1;
                break;
            }
            JOptionPane.showMessageDialog(null, p1.getPlayerName()
                + "'s Guess is Wrong!");
            if (p2.guessNumber()==numberToGuess){
                flagWinner = 2;
                break;
            }
            JOptionPane.showMessageDialog(null,p2.getPlayerName()
                + "'s Guess is Wrong!");
            if (p3.guessNumber()==numberToGuess){
                flagWinner = 3;
                break;
            }
            JOptionPane.showMessageDialog(null,p3.getPlayerName()
                + "'s Guess is Wrong!");
        }
        if (1 == flagWinner){
            JOptionPane.showMessageDialog(null,p1.getPlayerName()+ " Wins!");
        } else if (2 == flagWinner){
            JOptionPane.showMessageDialog(null,p2.getPlayerName()+ " Wins!");
        } else JOptionPane.showMessageDialog(null,p3.getPlayerName()+ " Wins!");
    }

    public static void main(String[] e){
        GuessGame gg = new GuessGame(
            JOptionPane.showInputDialog("Enter Player 1 Name: "),
            JOptionPane.showInputDialog("Enter Player 2 Name: "),
            JOptionPane.showInputDialog("Enter Player 3 Name: ")
        );
        gg.startGame();
    }
}

Stack Exchange Network

current community

your communities

more stack exchange communities

Guessing game - am I using the concept of objects in Java properly?

2 Answers 2

Your Answer

Hot Network Questions

Guessing game - am I using the concept of objects in Java properly?

2 Answers 2

Your Answer

Sign up or log in

Post as a guest

Related

Hot Network Questions