Rock Paper Scissors in one single method in Java - Good or bad?

Question

I'm new to programming and have created a simple Rock, Paper, Scissors game. The entire program is under a single class and the main method. I hear that's probably not the best way to code.

How should I have written this to utilize multiple classes or to adhere to best practice?

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

public class RPS {
    public static void main(String args[]){

    String choice, choice1;
    int num = 0;
    Random r = new Random();
    num = r.nextInt(3);

    System.out.println("Choose: Rock, Paper or Scissors.");
    Scanner userChoice = new Scanner(System.in);
    choice1 = userChoice.next();
    choice = choice1.toLowerCase();



    switch (num){
    case 0:
        String num1 = "rock";
        System.out.println("System chose Rock"); 
        if (choice.matches(num1)){
            System.out.println("Its a tie!");

            }
        else if (choice.matches("paper")){
            System.out.println("You win!");

        }
        else if (choice.matches("scissors")){
            System.out.println("You lose!");
        }
        else {
            System.out.println();
            System.out.println("ERROR: Please choose Rock, Paper or Scissors");
        }
    break;
    case 1:
        String num2 = "paper";
        System.out.println("System chose Paper");
        if (choice.matches(num2)){
            System.out.println("Its a tie!");

        }
        else if (choice.matches("scissors")){
            System.out.println("You win!");

        }
        else if (choice.matches("rock")){
            System.out.println("You lose!");
        }
        else {
            System.out.println();
            System.out.println("ERROR: Please choose Rock, Paper or Scissors");
        }
    break;
    case 2: 
        String num3 = "scissors";
        System.out.println("System chose Scissors");
        if (choice.matches(num3)){
            System.out.println("Its a tie!");

        }
        else if (choice.matches("rock")){
            System.out.println("You win!");

        }
        else if (choice.matches("paper")){
            System.out.println("You lose!");
        }
        else {
            System.out.println();
            System.out.println("ERROR: Please choose Rock, Paper or Scissors");
        }
    break;


    }

}
}

Answer 1

I would make some changes, although your code is quite simple.

First, get rid of the magic Strings for rock/paper/scissors, and create an enum.

I would possibly also move the logic of checking if it's a win/draw/loose inside that enum, so it would be much easier to add new entries (imagine you want to extend that to a Rock/Paper/Scissors/Lizard/Spock.

My main recomendation is to separate getting the input from the user, and the logic of the actual program.

If these two concepts are completely separated, you could reuse the logic to build, for instance, a web interface, or get the user input in any other way (or even get two inputs from two users, and use the same logic to decide who wins!).

Check this, for instance, for an enum representation of Rock/Paper/Scissors:

https://stackoverflow.com/a/9858163/432806

Overall I would advise you to have a structure like:

main:
    input = getUserInput
    enumValue = getEnumFromInput
    secondValue = getEnumFromComputerRoll
    testWhoWins(enumValue, secondValue) 

That last line could also be something like:

    enumValue.getResult(secondValue)

If you implement the logic inside the enum.

update

There's also another big downside to that monolithic approach, that hasn't been discussed yet: testing.

There is no simple way of writing unit tests for a program written in such a fashion.

To be able to have decent unit tests, you need to break up your programming logic into small pieces that can be tested independently from each other.

Think about:

• How would you test transforming a String (user input), to the correct Enum/String?
• How would you test generating a value in the right range, and converting into an Enum?
• How would you test the logic of, given two "moves", choosing the result?

I think this is a good way of figuring out how to break up your logic: Think about how you would write tests for it.

Answer 2

There is nothing inherently wrong about writing monolithic programs. There are lots of downsides, thought, most of them related to maintainability.

Your particular example can be refactored to make it more compact and probably easier to the eye. It can be also a good exercise on structures (here I'm using a String array of arrays, also known as a matrix.) For example:

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

public class RPS {
    public static void main(String args[]) {
        // Arrays of moves: move name, beaten by 
        String[][] moves = { { "rock", "paper", },
                { "scissors", "rock", },
                { "paper", "scissors", },
        };
        String choice;
        int num = 0;
        Random r = new Random();
        num = r.nextInt(3);

        System.out.print("Choose: ");
        for (int i = 0; i < moves.length; i++ ) {
            if (i > 0)
                System.out.print(", ");             
            System.out.print(moves[i][0]);
        }
        System.out.println(".");
        Scanner userChoice = new Scanner(System.in);
        choice = userChoice.next().toLowerCase();

        System.out.println("System chose " + moves[num][0]);
        if (choice.matches(moves[num][0])) {
            System.out.println("Its a tie!");
        } else if (choice.matches(moves[num][1])) {
            System.out.println("You win!");
        } else {
            System.out.println("You lose!");
        }
    }
}

A nice side effect of refactoring your code like this is that you can easily extend it. For example, two small modifications and you have the game Sheldon plays in A Big Bang Theory:

public class RPS {
    public static void main(String args[]) {
        // Arrays of moves: move name, beaten by, also beaten by
        String[][] moves = { { "rock", "paper", "spock" },
                { "scissors", "rock", "spock" },
                { "paper", "scissors", "lizard" },
                { "lizard", "rock", "scissors" },
                { "spock", "lizard", "paper" }, };
        String choice;
        int num = 0;
        Random r = new Random();
        num = r.nextInt(moves.length);

        System.out.print("Choose: ");
        for (int i = 0; i < moves.length; i++) {
            if (i > 0)
                System.out.print(", ");
            System.out.print(moves[i][0]);
        }
        System.out.println(".");
        Scanner userChoice = new Scanner(System.in);
        choice = userChoice.next().toLowerCase();

        System.out.println("System chose " + moves[num][0]);
        if (choice.matches(moves[num][0])) {
            System.out.println("Its a tie!");
        } else if (choice.matches(moves[num][1])
                || choice.matches(moves[num][2])) {
            System.out.println("You win!");
        } else {
            System.out.println("You lose!");
        }
    }
}