HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

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

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scissorspapermethodbadjavaonesinglerockgood

Problem

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

Solution

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.

Code Snippets

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

Context

StackExchange Code Review Q#25949, answer score: 7

Revisions (0)

No revisions yet.