Rock-Paper-Scissors

Question

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<Move, Move>    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 values of {@link Move} for the random generator
     */
    private static final Move[] moves   = Move.values();

    /**
     * The random number generator used; created once and then cached
     */
    private final Random        generator;

    public RandomPlayer() {
        generator = new Random();
    }

    @Override
    public Move getNextMove() {
        return moves[generator.nextInt(moves.length)];
    }

}

---

The game:

import java.lang.invoke.MethodHandles;
import java.util.EnumMap;
import java.util.Map;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
 * A Game of rock-scissor-paper
 * 
 * @author ms
 *
 */
public class Game {

    private static final int    NUMBER_OF_GAMES = 100;

    private static final Logger logger          = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

    /**
     * An enum encapsulating the results of one move which also holds the strings needed for
     * generating the output
     * 
     * @author ms
     *
     */
    enum Result {
        A_WINS("Player A wins "), B_WINS("Player B wins "), TIE("Tie: ");

        private final String    output;

        private Result(String output) {
            this.output = output;
        }

        public String getOutput() {
            return output;
        }
    }

    /**
     * @param args
     */
    public static void main(final String[] args) {

        final Player playerA = new PaperPlayer();
        final Player blayerB = new RandomPlayer();
        final Map<Result, Integer> results = new EnumMap<>(Result.class);

        initResults(results);
        playGame(playerA, blayerB, results);
        printResults(results);
    }

    /**
     * Initialize the {@code results} map with 0 values
     * 
     * @param results
     *            the inialized results map
     */
    private static void initResults(final Map<Result, Integer> results) {
        for (final Result r : Result.values()) {
            results.put(r, 0);
        }
    }

    /**
     * Plays {@link #NUMBER_OF_GAMES} rounds of the game
     * 
     * @param playerA
     *            A player
     * @param blayerB
     *            A player
     * @param results
     *            The results are stored here
     * @see #playOneRoundOfTheGame(Player, Player, Map)
     */
    private static void playGame(final Player playerA, final Player blayerB, final Map<Result, Integer> results) {
        for (int i = 0; i < NUMBER_OF_GAMES; i++ ) {
            playOneRoundOfTheGame(playerA, blayerB, results);
        }
    }

    /**
     * Plays one round of the game
     * 
     * @param playerA
     *            A player
     * @param blayerB
     *            A player
     * @param results
     *            The results are stored here
     * @see #playGame(Player, Player, Map)
     */
    private static void playOneRoundOfTheGame(final Player playerA, final Player blayerB,
            final Map<Result, Integer> results) {
        final Move moveA = playerA.getNextMove();
        final Move moveB = blayerB.getNextMove();
        final Result result = evaluateMoves(moveA, moveB);
        logger.debug("A: {}, B: {}. result: {}", moveA, moveB, result);
        results.put(result, results.get(result) + 1);
    }

    /**
     * Evaluates one round of the game
     * 
     * @param moveA
     *            The move of one player
     * @param moveB
     *            The move of another player
     * @return The {@link Result}
     */
    static Result evaluateMoves(final Move moveA, final Move moveB) {
        final Result result;
        if (Move.beats(moveA) == moveB) {
            result = Result.A_WINS;
        }
        else {
            if (Move.beats(moveB) == moveA) {
                result = Result.B_WINS;
            }
            else {
                result = Result.TIE;
            }
        }
        return result;
    }

    /**
     * Prints the results
     * 
     * @param results
     *            The results
     */
    private static void printResults(final Map<Result, Integer> results) {
        for (final Result r : Result.values()) {
            System.out.printf("%s %d of %d games%n", r.getOutput(), results.get(r), NUMBER_OF_GAMES);
        }
    }

}

---

A test:

import java.util.Arrays;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameter;
import org.junit.runners.Parameterized.Parameters;

import Game.Result;

import static org.assertj.core.api.Assertions.*;

/**
 * @author ms
 *
 */
@RunWith(Parameterized.class)
public class GameTest {

    @Parameters
    public static Iterable<Object[]> testData() {
        return Arrays.asList(new Object[][] { { Move.ROCK, Move.ROCK, Result.TIE },
                { Move.ROCK, Move.PAPER, Result.B_WINS }, { Move.ROCK, Move.SCISSORS, Result.A_WINS },
                { Move.PAPER, Move.PAPER, Result.TIE }, { Move.PAPER, Move.ROCK, Result.A_WINS },
                { Move.PAPER, Move.SCISSORS, Result.B_WINS }, { Move.SCISSORS, Move.SCISSORS, Result.TIE },
                { Move.SCISSORS, Move.ROCK, Result.B_WINS }, { Move.SCISSORS, Move.PAPER, Result.A_WINS }, });
    }

    @Parameter(0)
    public Move     moveA;

    @Parameter(1)
    public Move     moveB;

    @Parameter(2)
    public Result   expectedResult;

    /**
     * Test method for
     * {@link Game#evaluateMoves(Move, Move)}
     * .
     */
    @Test
    public void testEvaluateMoves() throws Exception {
        assertThat(Game.evaluateMoves(moveA, moveB)).isEqualTo(expectedResult);
    }

}

---

Pros:

• The logic of the game (which move beats which) is kept in one place (the definition of Move), making it easy to replace the rules with e.g. rock-paper-scissors-lizard-spock

Cons:

• The naming of the components feels too trivial

• The main loop in Game.playGame could probably be converted to using Java8 Streams/Lambdas, but this makes collecting the results more difficult

Only the evaluation of moves is tested, as this is the only non-trivial code

Answer 1

But I'm still interested in a Java8/Streams-solution. – @Martin Schröder

Hey... it's show-and-tell time!

Move

enum Move {
    ROCK, PAPER, SCISSORS;

    boolean beats(Move another) {
        switch (this) {
        case ROCK:
            return another == SCISSORS;
        case PAPER:
            return another == ROCK;
        case SCISSORS:
            return another == PAPER;
        // note: see alternative below
        default:
            throw new IllegalStateException();
        }
        // alternatively, just throw here without the default case
        // throw new IllegalStateException();
    }
}

As suggested by @tim and @OldCurmudgeon, it will be nicer if a Move knows whether it beats another Move, instead of relegating that to a static method. However, my method differs by going for a switch statement instead of relying on a static Map for two reasons:

• I find that the logic is clearer to read.
• Any good IDE will helpfully suggest you to add cases for all your enum values.

If Lizard and Spock paid a visit to your game, the Map solution will require significant re-tooling, whereas... it's at least easier to refactor the switch statement to prudently take care of them, for starters.

edit The alternative throw-ing of IllegalStateException suggested by @maaartinus is nifty in the sense that any good IDE will also prompt you to add case clauses for new enum values, and you'll still get the exception thrown.

Player

enum Player implements Iterator<Move> {
    A() {
        @Override
        public Move next() {
            return Move.PAPER;
        }
    }, B() {
        @Override
        public Move next() {
            return Move.values()[GENERATOR.nextInt(Move.values().length)];
        }
    };

    private static final Random GENERATOR = new Random();

    @Override
    public boolean hasNext() {
        return true;
    }

    @Override
    public void remove() {
        throw new UnsupportedOperationException();
    }
}

@OldCurmudgeon's answer actually had a revision on this implementation with the Iterator interface, but for some reason it's not there anymore. Anyways, I have chosen to model this as an enum since we know that there are usually only two players in RPS (discounting any hard-core MMORPS - I just made that up), and this is also explicitly defined in the question.

I could do a switch too inside a common next() method implementation, but I thought I'll just use an overriding approach for a change. The other thing of note is the throw-ing of UnsupportedOperationException() in the remove() method: this is just good form.

Round

private static final class Round {

    private final Move aMove;
    private final Move bMove;

    private Round(Move aMove, Move bMove) {
        this.aMove = aMove;
        this.bMove = bMove;
    }

    public Optional<Player> getWinner() {
        return aMove == bMove ? Optional.empty() :
            Optional.of(aMove.beats(bMove) ? Player.A : Player.B);
    }

    public static Round of(Player aPlayer, Player bPlayer) {
        if (aPlayer.hasNext() && bPlayer.hasNext()) {
            return new Round(aPlayer.next(), bPlayer.next());
        }
        throw new NoSuchElementException("No more moves by player "
                + (aPlayer.hasNext() ? Player.B : Player.A));
    }
}

Now this is where I differ significantly in the modeling, and also where the first Java 8 feature Optional shows up.

Your Result is merely a... result of applying a function to two moves to know who won. That means given a Result, one does not know what are the inputs resulting in the outcome. The shortcomings are:

• You have to model a Player and its representation as a Result separately (see PaperPlayer/Result.A_WINS and RandomPlayer/Result.B_WINS).
  • More code, more places to change if I want to change my player strategy, names, or mapping external inputs into moves.
• Once you capture the results, you lose the inputs.
  • Subsequently, you cannot replay the game.
  • It also becomes easy to lie in code since you cannot re-compute the scores.

Hence, I went with capturing the inputs of a round of the game, simply called Round. All it captures are both moves, and calling getWinner() then applies said function to give us an Optional<Player>.

One nifty feature of using Optional here is that it allows us to model a tri-state: Either player has won, or there is a tie. This allows me to conveniently do away with using null and doing null-checks explicitly, or having to add code for a tri-value enum to model a tie scenario. I will touch on this a little more later, and if you think about it, a tie scenario itself is also a result of neither player winning. Therefore, in finding out the number of ties in a game, there is no hard requirement to store that with a tie representation, since the value can be computed.

There is a light sanity check inside the static of() method, to make full use of our Player class implementing an Iterator interface.

One more thing about your solution: Your use of output for each of your Result values is really just the required prefix to use when you are displaying the results. There is therefore little flexibility should the requirements change: e.g. change in names or pattern.

Game

public final class Game {

    private static final int NUMBER_OF_GAMES = 100;
    private static final Logger logger = LoggerFactory.getLogger(Game.class);

    private static Map<Player, Long> play() {
        return Stream.generate(() -> Round.of(Player.A, Player.B))
                .limit(NUMBER_OF_GAMES)
                .map(Round::getWinner)
                .filter(Optional::isPresent)
                .collect(Collectors.groupingBy(Optional::get,
                        () -> new EnumMap<>(Player.class), Collectors.counting()));
    }

    public static void main(String[] args) {
        Map<Player, Long> results = Game.play();
        int wins = results.values().stream().mapToInt(Long::intValue).sum();
        results.forEach((k, v) -> logger.info("Player {} wins {} of {} games", k, v,
                Integer.valueOf(NUMBER_OF_GAMES)));
        logger.info("Tie: {} of {} games", Integer.valueOf(NUMBER_OF_GAMES - wins),
                Integer.valueOf(NUMBER_OF_GAMES));
    }
}

1. Stream.generate() supplies us with an infinite sequential stream of Rounds for us, which are constructed based on calling Round.of(Player, Player).
2. This is limited by calling limit(NUMBER_OF_GAMES).
3. We then have to determine our winners of each Round by calling, i.e. map()-ping, to its getWinner method reference.
4. Now, since this is actually an Optional wrapper, we can filter() by calling its isPresent() method (again, another method reference).
5. Finally, we collect() our winners by doing a groupingBy() with the appropriate arguments, remembering to call get() first on our Optional wrapper in order to access the underlying winning Player.

Inside our main() method, we can count the number of wins first after calling Game.play(). That is done by performing a sum() on the IntStream returned when we called mapToInt(). To display the required results, we only need to do a forEach() on the Map and 'consume' both the key and value accordingly. The last line displays the number of ties for us.

Answer 2

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);.

Answer 3

This is well-written and considered code.

I will echo the other's suggestions.

1. Make your Move enum have a beats method.
2. Make a Results object that maintains the results.

Here's what's left after I've tinkered with it.

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

    ROCK, PAPER, SCISSORS;

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

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

    /**
     * Returns true if this move beats
     *
     * @param other move to compare with.
     * 
     * @return true if this move beats the other.
     */
    public boolean beats(Move other) {
        return beats.get(this) == other;
    }
}

/**
 * The superclass of all players
 *
 * @author ms
 *
 */
public abstract static 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 static class PaperPlayer extends Player {

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

}

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

    /**
     * Caches all values of {@link Move} for the random generator
     */
    private static final Move[] moves = Move.values();

    /**
     * The random number generator used; created once and then cached
     */
    private final Random generator;

    public RandomPlayer() {
        generator = new Random();
    }

    @Override
    public Move getNextMove() {
        return moves[generator.nextInt(moves.length)];
    }

}

/**
 * A Game of rock-scissor-paper
 *
 * @author ms
 *
 */
public static class Game {

    private static final int NUMBER_OF_GAMES = 100;

    /**
     * An enum encapsulating the results of one move which also holds the strings needed for generating the output
     *
     * @author ms
     *
     */
    enum Result {

        A_WINS("Player A wins "), B_WINS("Player B wins "), TIE("Tie: ");

        private final String output;

        private Result(String output) {
            this.output = output;
        }

        public String getOutput() {
            return output;
        }
    }

    private static class Results {

        final ConcurrentMap<Result, Integer> results = new ConcurrentHashMap<>(new EnumMap<>(Result.class));

        private void result(Result result) {
            // Add one (to 0 if it's not present.
            results.put(result, results.computeIfAbsent(result, x -> 0) + 1);
        }

        @Override
        public String toString() {
            StringBuilder s = new StringBuilder();
            for (final Result r : Result.values()) {
                s.append(r.getOutput())
                        .append(results.get(r)).append(" of ")
                        .append(NUMBER_OF_GAMES)
                        .append("\r\n");
            }
            return s.toString();
        }
    }

    /**
     * @param args
     */
    public static void main(final String[] args) {

        final Player playerA = new PaperPlayer();
        final Player blayerB = new RandomPlayer();
        final Results results = new Results();

        playGame(playerA, blayerB, results);
        System.out.println(results);
    }

    /**
     * Plays {@link #NUMBER_OF_GAMES} rounds of the game
     *
     * @param playerA A player
     * @param blayerB A player
     * @param results The results are stored here
     * @see #playOneRoundOfTheGame(Player, Player, Map)
     */
    private static void playGame(final Player playerA, final Player blayerB, Results results) {
        for (int i = 0; i < NUMBER_OF_GAMES; i++) {
            playOneRoundOfTheGame(playerA, blayerB, results);
        }
    }

    /**
     * Plays one round of the game
     *
     * @param playerA A player
     * @param blayerB A player
     * @param results The results are stored here
     * @see #playGame(Player, Player, Map)
     */
    private static void playOneRoundOfTheGame(final Player playerA, final Player blayerB,
            Results results) {
        final Move moveA = playerA.getNextMove();
        final Move moveB = blayerB.getNextMove();
        final Result result = evaluateMoves(moveA, moveB);
        //logger.debug("A: {}, B: {}. result: {}", moveA, moveB, result);
        results.result(result);
    }

    /**
     * Evaluates one round of the game
     *
     * @param moveA The move of one player
     * @param moveB The move of another player
     * @return The {@link Result}
     */
    static Result evaluateMoves(final Move moveA, final Move moveB) {
        final Result result;
        if (moveA.beats(moveB)) {
            result = Result.A_WINS;
        } else if (moveB.beats(moveA)) {
            result = Result.B_WINS;
        } else {
            result = Result.TIE;
        }
        return result;
    }
}

public void test() {
    Game.main(null);
}

I am no expert in streams so this may not be an ideal solution but here's a moderately streamed version.

Essentially, the game is a stream of Play objects which are evaluated, grouped by the result and counted into a Map.

Most of the streaming is in the playGame method.

/**
 * A Game of rock-scissor-paper
 *
 * @author ms
 *
 */
public static class Game {

    private static final int NUMBER_OF_GAMES = 100;

    /**
     * An enum encapsulating the results of one move which also holds the strings needed for generating the output
     *
     * @author ms
     *
     */
    enum Result {

        A_WINS("Player A wins "), B_WINS("Player B wins "), TIE("Tie: ");

        private final String output;

        private Result(String output) {
            this.output = output;
        }

        public String getOutput() {
            return output;
        }
    }

    /**
     * @param args
     */
    public static void main(final String[] args) {

        final Player playerA = new PaperPlayer();
        final Player blayerB = new RandomPlayer();

        playGame(playerA, blayerB);
    }

    /**
     * Represents one play of the game.
     */
    private static class Play {

        final Move a;
        final Move b;

        public Play(Move a, Move b) {
            this.a = a;
            this.b = b;
        }

    }

    /**
     * Evaluates one round of the game
     *
     * @param moveA The move of one player
     * @param moveB The move of another player
     * @return The {@link Result}
     */
    static Result evaluateMove(final Move moveA, final Move moveB) {
        final Result result;
        if (moveA.beats(moveB)) {
            result = Result.A_WINS;
        } else if (moveB.beats(moveA)) {
            result = Result.B_WINS;
        } else {
            result = Result.TIE;
        }
        return result;
    }

    /**
     * Plays {@link #NUMBER_OF_GAMES} rounds of the game
     *
     * @param playerA A player
     * @param playerB A player
     * @param results The results are stored here
     */
    private static void playGame(final Player playerA, final Player playerB) {
        Map<Result, Long> results = IntStream.range(0, NUMBER_OF_GAMES)
                // Make one play per round.
                .mapToObj(i -> new Play(playerA.next(), playerB.next()))
                // Evaluate that move to a Result
                .map(p -> evaluateMove(p.a, p.b))
                // Count each result
                .collect(Collectors.groupingBy(r -> r, Collectors.counting()));
        // Print them.
        printResults(results);
    }

    /**
     * Prints out the results.
     *
     * @param results
     */
    private static void printResults(Map<Result, Long> results) {
        StringBuilder s = new StringBuilder();
        for (final Result r : Result.values()) {
            s.append(r.getOutput())
                    .append(results.get(r)).append(" of ")
                    .append(NUMBER_OF_GAMES)
                    .append("\r\n");
        }
        System.out.println(s);
    }

}

public void test() {
    Game.main(null);
}

And your plan worked! This works fine.

public enum Move {

    ROCK, PAPER, SCISSORS, LIZARD, SPOCK;

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

    // init the beats
    static {
        // Smashes scisors, Vaporizes rock
        beats.put(SPOCK, EnumSet.of(SCISSORS, ROCK));
        // Cuts paper, Decapitates lizard
        beats.put(SCISSORS, EnumSet.of(PAPER, LIZARD));
        // Covers rock, Disproves Spock
        beats.put(PAPER, EnumSet.of(ROCK, SPOCK));
        // Crushes lizard, Breaks scisors
        beats.put(ROCK, EnumSet.of(LIZARD, SCISSORS));
        // Poisons Spock, Eats paper 
        beats.put(LIZARD, EnumSet.of(SPOCK, PAPER));
    }

    /**
     * Returns true if this move beats
     *
     * @param other move to compare with.
     *
     * @return true if this move beats the other.
     */
    public boolean beats(Move other) {
        return beats.get(this).contains(other);
    }
}

Answer 4

Your solution is nice, but it could be much shorter and expressed as a function

• Enumerate the choices from 0-2 (0: rock, 1: paper, 2: scissors)
• Enumerate the scores column from 0-2 (0: Tie, 1: Player A, 2: Player B)

And voila, the function to determine where the point for the round should go:

int [] choices = {0,1,2}; // Rock, Paper, Scissors
int [] players = {0,1,2}; // Tie, Player A, Player B
int [] scores = {0,0,0};  // Scores table

int playerAChoice = ...; //get input from player A
int playerBChoice = ...; //get input from player B

//3 choices with wraparound effect
int winner_index = (3 + playerAChoice - playerBChoice) % 3 

scores[winner_index] += 1;

Edit: Explanation of why I chose to suggest this solution instead of enums. Rock paper scissors is a subtype of a balanced game type where each weapon (i.e. rock/paper/scissors) beats the same amount of weapons as all other weapons. This can be expanded with "lizard spock" for instance.

If you were to model this with enums...it will be a tedious and error-prone process. You would have to literally copy/paste the rules for each game and it would also not be flexible.

Now, if you were to compute this with math, this is much simpler. Actually, the rule I specified for rock-paper-scissors can be generalised even further. The rule for any such game can be modelled as follows (source: wikipedia on additional weapons in roshambo):

Alternatively, the rankings in rock-paper-scissors-Spock-lizard may be modeled 
by a comparison of the parity of the two choices. If it is the same (two odd
numbered moves or two even-numbered ones) then the lower number wins, while if
they are different (one odd and one even) the higher wins. 

Enums have their place in Java, but so does insight in the problem you are actually solving.