Rock Paper Scissors game using OOP

Question

I'm a complete beginner to Java and we have just started learning objects and classes in school. I'm trying to create a very simple text-based Rock Paper Scissors game. I do realise that my code is a mess, so I'm asking for some suggestions on how could I improve it and if I'm even in the right direction with OOP.

Main class:

public class Main {
 public static void main(String[] args) {
 new Game();
 }
}

Player class

public abstract class Player {
 private String name;
 private String choice;
 public Player() {}
 public Player(String name) {this.name = name;}
 public void setName(String newName) {
 name = newName;
 }
 public String getName() {
 return name;
 }
 public String getChoice() {
 return choice;
 }
 public void setChoice(String newChoice) {
 choice = newChoice;
 }
 public abstract void selectChoice();
}

User class

import java.util.Scanner;
public class User extends Player {
 private Scanner input;
 public User() {
 input = new Scanner(System.in);
 }
 public void selectChoice() {
 System.out.println("Enter your choice: R - Rock, P - Paper, S - Scissors");
 setChoice(input.nextLine().toUpperCase());
 }
}

Computer class

import java.util.Random;
public class Computer extends Player {
 private Random rand;
 private final int MAX_NUMBER = 3;
 public Computer() {
 setName("Computer");
 rand = new Random();
 }
 public void selectChoice() {
 int randomNumber = rand.nextInt(MAX_NUMBER);
 switch(randomNumber) {
 case 0:
 setChoice("ROCK");
 break;
 case 1:
 setChoice("PAPER");
 break;
 case 2:
 setChoice("SCISSORS");
 break;
 }
 }
}

Game class

import java.util.Scanner;
public class Game {
 private User p;
 private Computer com;
 private int playerWins;
 private int playerLoses;
 private int ties;
 private boolean isRunning = false;
 private Scanner scan;
 public Game() {
 p = new User();
 com = new Computer();
 scan = new Scanner(System.in);
 start();
 }
 private void start() {
 isRunning = true;
 System.out.println("Please, enter your name:");
 p.setName(scan.nextLine());
 while(isRunning) {
 displayScore();
 p.selectChoice();
 com.selectChoice();
 displayChoices();
 displayWinner(decideWinner());
 updateScore(decideWinner());
 playAgain();
 }
 }
 private void displayScore() {
 System.out.println(p.getName());
 System.out.println("----------");
 System.out.println("Wins: " + playerWins);
 System.out.println("Loses: " + playerLoses);
 System.out.println("Ties: " + ties);
 System.out.println("----------");
 }
 private int decideWinner() {
 // 0 - User wins
 // 1 - Computer wins
 // 2 - tie
 if(p.getChoice().equals("ROCK") && com.getChoice().equals("SCISSORS"))
 return 0;
 else if(p.getChoice().equals("PAPER") && com.getChoice().equals("ROCK"))
 return 0;
 else if(p.getChoice().equals("SCISSORS") && com.getChoice().equals("PAPER"))
 return 0;
 else if(com.getChoice().equals("ROCK") && p.getChoice().equals("SCISSORS"))
 return 1;
 else if(com.getChoice().equals("PAPER") && p.getChoice().equals("ROCK"))
 return 1;
 else if(com.getChoice().equals("SCISSORS") && p.getChoice().equals("PAPER"))
 return 1;
 else
 return 2;
 }
 private void displayChoices() {
 System.out.println("User has selected: " + p.getChoice());
 System.out.println("Computer has selected: " + com.getChoice());
 }
 private void displayWinner(int winner) {
 switch(winner) {
 case 0:
 System.out.println("User has won!");
 break;
 case 1:
 System.out.println("Computer has won!");
 break;
 case 2:
 System.out.println("It is a tie!");
 }
 }
 private void playAgain() {
 String choice;
 System.out.println("Do you want to play again? Enter Yes to play again.");
 choice = scan.nextLine();
 if(!(choice.toUpperCase().equals("YES") ))
 isRunning = false;
 }
 private void updateScore(int winner) {
 if(winner == 0)
 playerWins++;
 else if(winner == 1)
 playerLoses++;
 else if(winner == 2)
 ties++;
 }
}

• jpmc26: What problem do you expect an OOP approach to solve for you? Design decisions should never be made in a vacuum.
• Ivo: I suggest you do some validation on the user's input. If the user types anything but the 3 options it always results in a tie now. And you also misleadingly instruct the user to input R, P or S while in fact he needs to type it out
• IMil: @jpmc26 come on, it's a programming assignment. You might as well ask what problem is solved by making computer write "Hello world".
• jpmc26: @IMil If it's an assignment, that's even more reason to focus on what the tool is best used for rather than apply it blindly. During coursework is the time you're supposed to be learning. Sadly, our discipline does a very poor job in the classroom. It teaches platitudes and incorrect absolutes, leaving new recruits a net cost for employers and struggling to learn the basics on the job instead.
• Marco13: @jpmc26 I once had a similar assignment. Not in a freshman course, but as part of the hiring process for a senior software developer position that I applied for (with ~20 years of programming experience). You can implement "rock-paper-scissors" with maybe 50 LOC. That's not the point. (I'll probably post my code from back then as part of an answer. I didn't want the job anyhow.)

Answer 1

Good job so far!

The previous answers already cover a lot, I just want to add this:

Don't use String for state!

Currently you use String to encode the Players Choice. This is hard to refactor and it is difficult to add functionality. Prefer its own Class or Enum.

For example

 enum Choice { ROCK, PAPER, SCISSORS}

Then, you can have a variable containing the choice, for example:

 Choice choice = Choice.ROCK;

You can add behaviour to the enum:

enum Choice { 
 ROCK, PAPER, SCISSORS
 public boolean beats(Choice other)
 {
 switch (this)
 { 
 case ROCK:
 return other == SCISSORS;
 case PAPER:
 return other == ROCK;
 case SCISSORS:
 return other == PAPER;
 }
 }
}

Now, your other code can just call myChoice.beats(otherChoice).

Also, you can implement random() in the enum, as well as parseFromUserInput(String s). If you do this correctly, you can change the enum, adding different choices, without needing to change any other code in your application.

So, if you want to implement RockPaperScissorsLizardSpock , you just extend the Enum, and be done!

Also:

Use Enum instead of magic int values

Instead of returning int that encodes the result of the round, use a explicit enum as well, replacing the boolean beats() method above:

enum Result { WIN, LOSS, DRAW }
public Result fights(Choice other)
 {
 switch (this)
 { 
 case ROCK:
 if (other == ROCK)
 return Result.DRAW
 return other == SCISSORS ? Result.WIN : Result.LOSS;
 ....
 }
 }

Answer 2

This code really isn't a mess! While there are a few relatively minor points I can make, your code is generally easy to read and understand, and there's nothing horribly wrong about it.

1. Watch out for user input! I can enter Z (or anything but rock/paper/scissors) and the result will always be a tie. You should tell the user when they enter something invalid and have them correct it.

2. Both Computer and Player use setName to set their name - once. I would prefer requiring the use of the Player constructor with the name parameter since you could then mark name as final.

3. Why is setChoice public? It is (and should) only used by selectChoice, so mark it protected.

4. What happens when I want to play a game between two computer opponents (or two people)? Right now I can't. It would be nice if Game let me pass the two Player objects in that will be opponents.

5. While I appreciate the MAX_NUMBER constant in the Computer class instead of just including a magic number, it's a better idea to store your choices in an array and then select a random element of the array.

6. Passing around strings for choices isn't so bad when just dealing with Rock/Paper/Scissors, but it will quickly become unmanagable. Enum Types were introduced to fix this problem, and generally do a pretty good job of it! It wouldn't hurt to start using them now. Another good spot for an enumeration is in the decideWinner method.

   The Choice enum type could also provide a method winsAgainst(Choice other) that can be used to simplify decideWinner.

7. For the most part, your names are great... until we get to Game and see p and com.

8. It would be nice to use the user's name instead of User when displaying the winner and when displaying choices.

The following points may directly contradict what your teacher says, as they are more opinion based. Follow whatever your team's (or assignment's) guidelines say.

1. Don't be afraid to include more than one line of code in main. If I wrote this program my main function would look something like this:

   Scanner scanner = new Scanner(System.in);
   System.out.println("Please enter your name:");
   Player user = new User(scanner.nextLine());
   Player computer = new Computer();
   Game game = new Game(user, computer);
   do {
    game.play();
    System.out.println("Play again? [Yes/No]");
   } while (scanner.nextLine().toUpperCase().equals("YES"));
   

2. Use braces on all if statements that aren't one line long. I would be fine with if (winner == 0) playerWins++;, but if (winner == 0)\n playerWins++; is much easier to mess up, especially if you don't have automatic indentation.

3. Don't declare variables before initializing them, if possible. There's no reason to declare choice before printing out instructions in the playAgain method.

• JollyJoker: Enums (or constants) avoid spelling errors. You don't get a clear idea of what's wrong from an error message if you just write "SCISORS" once.
• IMil: BTW this part should be emphasized: Player computer = new Computer(). The OP had them declared as derived types, which is... ok, but inflexible.