Play a game of Rock, Paper, Scissors

I just made a Rock, Paper, Scissors game and I have read some of the previous threads about them. Just wondering if there was any advice to optimize or just suggestions for naming or doing something better for my code. I have learned Java for only about 2 weeks so bear with me if I don't understand some of the functions.

I read somewhere in another thread that I could use a method to print out blocks of text if they were repetitive. I'm wondering if I could implement that (if it's actually more efficient) in my game.

Pastebin link for better viewing: http://pastebin.com/RbpaA4q2

```
import java.util.Random;
import java.util.Scanner;

public class Application {

public static void main(String[] args) {

boolean validMove;

int playerScore = 0;
int computerScore = 0;

String newLineSpacing = "\n---------------";

do {System.out.println("Do you pick rock, paper or scissors?");

Scanner choice = new Scanner(System.in);

// The next line that the player types is the move he is making
String playerChoice = choice.nextLine();

validMove = true;

if (!("rock".equals(playerChoice) ||
"paper".equals(playerChoice) ||
"scissors".equals(playerChoice))) {
System.out.println("Move not recognized. Try again by running the program again.");
validMove = false;
} else {

Random randomNumber = new Random();
// Random number from 0 - 2 (0, 1, 2)
int computerNumber = randomNumber.nextInt(2);

// Computer chooses a move based on a randomly generated integer
// 0 = rock
// 1 = paper
// 2 = scissors
String computerMoveChoice;
if (computerNumber == 0) {
computerMoveChoice = "rock";
} else if (computerNumber == 1) {

Bugs

First of all, there are 2 bugs:

-
In the code determining the choice by the computer, you have

int computerNumber = randomNumber.nextInt(2);

However, nextInt(2) will not return a random number between 0 and 2 inclusive, but 0 and 2 exclusive. As such, this method will return either 0 or 1. You need to call nextInt(3) to return a random number in the interval [0,2].

-
If both choice are the same, you are comparing the two Strings with

if (playerChoice == computerMoveChoice) {
    // ...
}

As you have done in the rest (so I guess this is more a typo), Strings should be compared with equals and not ==.

To the refactorings!

Computer choice

What you're missing are methods. All of the code is inside the main method: it would be preferable to create dedicated methods to make the code a lot clearer. Let's to this part by part.

The choice of the computer can be safely extracted into a method getComputerChoice(). Taking directly your code, it would look like:

private static String getComputerChoice() {
    Random randomNumber = new Random();
    // Random number from 0 - 2 (0, 1, 2)
    int computerNumber = randomNumber.nextInt(3);

    // Computer chooses a move based on a randomly generated integer
    // 0 = rock
    // 1 = paper
    // 2 = scissors
    String computerMoveChoice;
    if (computerNumber == 0) {
        computerMoveChoice = "rock";
    } else if (computerNumber == 1) {
        computerMoveChoice = "paper";
    } else {
        computerMoveChoice = "scissors";
    }
    return computerMoveChoice;
}

That can be improved a lot.

• This method is creating a new Random object each time it is invoked. This is not a good idea and leads to very poor random numbers. A Random object should be created just once and shared by the rest of the application. In this case, it can be made a constant of the game with private static final Random RANDOM = new Random();.

• To return the choice of the computer, an if / else if is performed on the possible random integers generated. Instead, consider having a list of the possible values, and returning a random element from the list.

With those two changes, you have:

private static final Random RANDOM = new Random();
private static final List CHOICES = Arrays.asList("rock", "paper", "scissors");

private static String getComputerChoice() {
    return CHOICES.get(RANDOM.nextInt(CHOICES.size()));
}

We were able to break it down into a very clear single line of code. Note also that we don't hardcode the number of choice anymore (like we did before with nextInt(3)). If tomorrow, we add another choice, it will be an easy change to the CHOICES list.

Player choice

Just like the computer choice, the choice of the player should be extracted inside its own dedicated method.

private static String getPlayerChoice() {
    System.out.println("Do you pick rock, paper or scissors?");
    Scanner choice = new Scanner(System.in);
    String playerChoice = choice.nextLine();
    if (CHOICES.contains(playerChoice)) {
        return playerChoice;
    }
    throw new InvalidChoiceException(playerChoice);
}

The method asks the player their choice. If it's a correct choice, it returns it. To determine whether it is correct, we don't rely on the hard-coded list of choices, we can reuse the CHOICES list made before, which now makes this method generic.

What happens in case it isn't correct can be discussed: you can return null but I find clearer to throw a custom exception: the user entering garbage qualifies as an exceptional condition that getPlayerChoice shouldn't need to handle. It is up to the game to decide what to do in that case. The exception takes the incorrect choice as parameter, in case we want to log it later on.

With this, the rest of the code needs to be changed accordingly. First, validMove doesn't make sense anymore: this was effectively replaced by the exception InvalidChoiceException. We can now have:

while (true) {
    String playerChoice, computerMoveChoice;
    try {
        playerChoice = getPlayerChoice();
        computerMoveChoice = getComputerChoice();
    } catch (InvalidChoiceException e) {
        System.out.println("Move not recognized. Try again by running the program again.");
        break;
    }
    // .. rest of code here
}

Notice that InvalidChoiceException could, in theory, be thrown by the player or the computer, so it makes sense to try both of the calls.

The winner / loser logic

So far, the logic to determine who wins the game (which relies on a lot of comparison between Strings), the logic to print the correct message and the logic to update the scores, are intermingled.

The first issue is comparing raw Strings to determine who is winning. This is error-prone and it is better to use an enum for all the possible moves:

```
private static enum Choice {

ROCK, PAPER, SCISSORS;

public boolean beats(Choice choice) {
switch (

StackExchange Code Review Q#138225, answer score: 3