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

Rock Paper Scissors Lizard Spock as a code-style and challenge

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

Problem

The following is a program that lets a Human play "Rock Paper Scissors Lizard Spock" against the computer... (almost playable at: http://ideone.com/EBDlga)

Note I have posted a follow-up question incorporating the suggestions that have been made in answers and comments on this question.

Comments, critiques, suggestions are welcome and encouraged.

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

/**
* Rock Paper Scissors Lizard Spock
*
* http://en.wikipedia.org/wiki/Rock-paper-scissors-lizard-Spock
*
* Interface for a human to play against the computer.
*/
public class RPSLS {

/**
* Set up the rules for the game.
* What are the moves, and what beats what!
*/
public enum Move {
Rock, Paper, Scissors, Lizard, Spock;

static {
Rock.willBeat(Scissors, Lizard);
Paper.willBeat(Rock,Spock);
Scissors.willBeat(Paper, Lizard);
Lizard.willBeat(Spock,Paper);
Spock.willBeat(Rock,Scissors);
}

// what will this move beat - populated in static initializer
private Move[] ibeat;
private void willBeat(Move...moves) {
ibeat = moves;
}

/**
* Return true if this Move will beat the supplied move
* @param move the move we hope to beat
* @return true if we beat that move.
*/
public boolean beats(Move move) {
// use binary search in case someone wants to set up crazy rules.
return Arrays.binarySearch(ibeat, move) >= 0;
}

}

// This is a prompt that is set up just once per JVM
private static final String MOVEPROMPT = buildOptions();
private static String buildOptions() {
StringBuilder sb = new StringBuilder();
sb.append(" -> ");
// go through the possible moves, and make a prompt string.
for (Move m : Move.values()) {
sb.append(m.ordinal() + 1).appen

Solution

-
You seem to like static things, but this gives me a feeling that your code is more procedural-oriented than object-oriented.

-
Although not an issue in your actual implementation, I think willBeat should call Arrays.copyOf to create a copy of the input. If you would call someMove.willBeat(someArray) and then later change the indexes in someArray, you would change the rules of the game.

-
The documentation of Arrays.binarySearch states:


The array must be sorted (as by the sort(byte[]) method) prior to making this call. If it is not sorted, the results are undefined

Your code does not guarantee that the items are sorted. (It might work correctly anyway since your arrays only contains two items, but it still feels wrong) The values for Lizard are not sorted in the same way the others are.

-
You are using htotal and ctotal, hscore and cscore, Move computer and Move human. Assuming htotal stands for "Human Total", player classes would be useful here.

-
2 and 3 are very Magic numbers.

-
The variable names htotal and hscore is a bit confusing.

-
Your main method is quite long and does a lot of things. Some of these things could be extracted into other methods/objects. Your main method currently is responsible for:

  • Keeping score



  • Keeping score (of a single "best of three")



  • Randomize a computer move



  • Determine who wins a game



-
I like your playAgain method and the way you are handling user input-output.

-
Overall, your code is not very abstracted. It seems to work, and it seems to do it's job well though.

Context

StackExchange Code Review Q#36394, answer score: 9

Revisions (0)

No revisions yet.