Rock Paper Scissors abstraction in Python

Question

I did a senior Python backend engineer interview that involved implementing a Rock Paper Scissors game. The result was not pretty. Their engineer said I have clumsy implementation all over the place. I did an upgrade afterwards, but still have one issue that confuses me. They would like me to change my original implementation of the game rules to a better abstraction.

My original implementation is:

def _outcome(self, player_move, ai_move):
    """Determine the outcome of current round.

    Paper beats (wraps) rock.
    Rock beats (blunts) scissors.
    Scissors beats (cuts) paper.

    Parameters
    ----------
    player_move : MoveChoice
        Player's move for current round.

    ai_move : MoveChoice
        AI's move for current round.

    Returns
    -------
    outcome : Outcome
        Outcome of the current round.
    """
    if player_move == 1 and ai_move == 2 or \
        player_move == 2 and ai_move == 3 or \
        player_move == 3 and ai_move == 1:
        return self.computer.name
    elif player_move == 1 and ai_move == 3 or \
        player_move == 2 and ai_move == 1 or \
        player_move == 3 and ai_move == 2:
        return self.player.name
    else:
        return 'Draw'

with a mapping:

MOVE_CHOICE = {
    1: 'rock',
    2: 'paper',
    3: 'scissors',
}

I have a role.Player and role.Computer class defined elsewhere. Their critiques are:

• Use of strings (player name or “draw”) to represent the outcome of a round
• No abstraction of the game rules, just a couple of big Boolean expression testing all combinations 1 by 1

Their suggestions are:

• Modelling moves (rock, paper, scissors) as an Enum
• Modelling round outcomes (win, lose, draw) as an Enum
• Modelling the games rules as either a function that takes 2 moves and returns an outcome, or as a dict mapping a move to a list of moves over which it wins

So far I've created Enums for Moves and Outcome:

class MoveChoice(Enum):
    ROCK = auto()
    PAPER = auto()
    SCISSORS = auto()


class Outcome(Enum):
    WIN = auto()
    LOSE = auto()
    DRAW = auto()

But I'm having trouble abstracting the game rules like they asked. How would I do that?

Answer 1

Based on the code you've shown and the recommendations they gave you, I would do something like this:

from enum import Enum, auto

class MoveChoice(Enum):
    ROCK = auto()
    PAPER = auto()
    SCISSORS = auto()

class Outcome(Enum):
    WIN = auto()
    LOSE = auto()
    DRAW = auto()

WIN_MAPPING = {
    MoveChoice.ROCK: MoveChoice.SCISSORS,
    MoveChoice.SCISSORS: MoveChoice.PAPER,
    MoveChoice.PAPER: MoveChoice.ROCK,
}

def outcome(player_move, ai_move):
    if player_move == ai_move:
        return Outcome.DRAW
    elif WIN_MAPPING[player_move] == ai_move:
        return Outcome.WIN
    else:
        return Outcome.LOSE

Note that I've changed your method to a function. This isn't a recommendation on overall program structure, it's just to present the logic as a more self-contained example.

---

As Luke Nelson pointed out in the comments, the interviewers suggested that the win mapping map each move to a list of moves it beats. I missed this detail. While it doesn't make any practical difference in Rock Paper Scissors, it is more generalized, and would make it easier if they then ask you to expand the rule set to something like Rock Paper Scissors Lizard Spock.

The only thing that would change about the mapping would be turning each value into a single-element list:

WIN_MAPPING = {
    MoveChoice.ROCK: [MoveChoice.SCISSORS],
    MoveChoice.SCISSORS: [MoveChoice.PAPER],
    MoveChoice.PAPER: [MoveChoice.ROCK],
}

Then the outcome function, instead of checking if the computer's move equals the value, would have to check if it's in the value (since the value is now a list).

def outcome(player_move, ai_move):
    if player_move == ai_move:
        return Outcome.DRAW
    # elif WIN_MAPPING[player_move] == ai_move:
    # Above line is replaced with:
    elif ai_move in WIN_MAPPING[player_move]:
        return Outcome.WIN
    else:
        return Outcome.LOSE