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

Rock-Paper-Scissors

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

Problem

I was given this exercise:


Build a game in which two players compete in a game of
Rock-Paper-Scissors with different strategies. Who will win more
rounds? The rules:



  • Scissors beats Paper



  • Rock beat Scissors



  • Paper beats Rock



  • If both players choose the same, the round is counted as a tie.





Implement two players:



  • Player A always chooses Paper



  • Player B chooses randomly





The game consists of 100 rounds of above two players competing. The
output of the program should be like the following:

"Player A wins 31 of 100 games"
"Player B wins 37 of 100 games"
"Tie: 32 of 100 games"


Here is my solution:

The moves:

import java.util.EnumMap;
import java.util.Map;

/**
 * The moves of a {@link Game}
 * 
 * @author ms
 *
 */
public enum Move {

    ROCK, PAPER, SCISSORS;

    /**
     * Holds the moves a move beats
     */
    private static final Map    beats   = new EnumMap<>(Move.class);

    // init the beats
    static {
        beats.put(ROCK, SCISSORS);
        beats.put(PAPER, ROCK);
        beats.put(SCISSORS, PAPER);
    }

    /**
     * Returns the move this move beats
     * 
     * @param m
     *            The current move
     * @return The move this move beats
     */
    public static Move beats(final Move m) {
        return beats.get(m);
    }

}


The players:

/**
 * The superclass of all players
 * 
 * @author ms
 *
 */
public abstract class Player {

    /**
     * Generates the next move
     * 
     * @return the next move
     */
    public abstract Move getNextMove();

}


/**
 * A player that always returns a {@link Move#PAPER} move
 * 
 * @author ms
 *
 */
public class PaperPlayer extends Player {

    @Override
    public Move getNextMove() {
        return Move.PAPER;
    }

}


```
import java.util.Random;

/**
* A player that always returns a random move
*
* @author ms
*
*/
public class RandomPlayer extends Player {

/**
* Caches all va

Solution

I like your code, it's mostly very readable and well structured.

Your evaluateMoves can be written a lot shorter by having multiple returns, and by using if-elseif instead of if-else-if:

static Result evaluateMoves(final Move moveA, final Move moveB) {
    if (Move.beats(moveA) == moveB) {
        return Result.A_WINS;
    } else if (Move.beats(moveB) == moveA) {
        return Result.B_WINS;
    } else {
        return Result.TIE;
    }
}


I would also think about restructuring your Move enum, so that you can write the much nicer if (moveA.beats(moveB)). This would also allow for easier addition of other objects than rock, paper, and stone.

I would also probably move the counting of results outside of playOneRoundOfTheGame (because it's not playing, it's counting). Either move it to the calling class, or - for cleaner code - create a separate Result class, which contains all the code that's currently mixed in with Game (such as the initialization, the adding, etc). Something like result.addWin(Player) would be a lot nicer than results.put(result, results.get(result) + 1);.

Code Snippets

static Result evaluateMoves(final Move moveA, final Move moveB) {
    if (Move.beats(moveA) == moveB) {
        return Result.A_WINS;
    } else if (Move.beats(moveB) == moveA) {
        return Result.B_WINS;
    } else {
        return Result.TIE;
    }
}

Context

StackExchange Code Review Q#90501, answer score: 10

Revisions (0)

No revisions yet.