Rock paper scissors coding assignment

Question

I had a coding assignment to write a Rock Paper Scissors game. The main task is not to show a solution but rather to test the coding style. The rules are such:

Problem:

rock is "O", paper is "[]", scissors is "8<". George and John play RPC(rock, paper and scissors) a couple of times. I receive 2 input strings containing their actions. it is guaranteed that both strings contain the same number of actions and the actions will be in {'O', '[]', '8<'}. Each game gets scored. If a player wins, he gets 2 points, if the game is draw, both get 1 point. the program has to calculate which player has how many points and what is the most common match up and how many times it occured.

Example:

input:

OOOOOO[][]8<

[][]8<8<8<O[]O[]

output:

George 12 John 6 O8< 3

explanation:

George plays O John plays [] the game outcome is O[] George points: 0 John points: 2

---

George plays O John plays [] the game outcome is O[] George points: 0 John points: 4

---

George plays O John plays 8< the game outcome is O8< George points: 2 John points: 4

---

George plays O John plays 8< the game outcome is O8< George points: 4 John points: 4

---

George plays O John plays 8< the game outcome is O8< George points: 6 John points: 4

---

George plays O John plays O the game outcome is OO George points: 7 John points: 5

---

George plays [] John plays [] the game outcome is [][] George points: 8 John points: 6

---

George plays [] John plays O the game outcome is []O George points: 10 John points: 6

---

George plays 8< John plays [] the game outcome is 8<[] George points: 12 John points: 6

---

most common match up is rock vs scissors and it happened 3 times. Final output:

George 12 John 6 O8< 3

My thoughts and code:

Thoughts:

Initially I made the solution in plain functions. After that I thought that I should wrap the code into classes, because it would be good to have variables like george's points available between different functions. I did not add any validators for the input data. I am not sure if that was correct or not. In the statement it was clearly explained that the input data will be passed correctly. I did not thought on edge cases like having empty inputs. The code that calculates who won the game maybe looks hardcoded as it again relies on correct input.

Code:

SCISSORS = "8<"
PAPER = "[]"
ROCK = "O"
GEORGE_LOSES = ["8<0", "O[]", "[]8<"]

class Player:
    def __init__(self, hands="") -> None:
        self.hands = hands
        self.index = 0
        self.hand_len = len(self.hands)
        self.points = 0
        self.current_hand = ""

    def has_hands(self):
        return self.index < self.hand_len

class RPS_solver:
    def __init__(self, george=Player(), john=Player()) -> None:
        self.george = george
        self.john = john
        self.george_result = ""
        self.john_result = ""
        self.matchup_outcome = ""
        self.matchup_dict = {"default": 0}
        self.most_common_matchup = "default"

    def next_item(self, player):
        if(player.hands[player.index] == "O"):
            player.index += 1
            return ROCK 
        if(player.hands[player.index] == "8"):
            player.index += 2
            return SCISSORS 
        else:
            player.index += 2
            return PAPER

    def count_match_points(self):
        if self.george.current_hand == self.john.current_hand:
            self.george.points += 1
            self.john.points += 1

        elif self.matchup_outcome in GEORGE_LOSES:
             self.john.points += 2

        # assuming all input data is correct
        else:
             self.george.points += 2

    def get_most_common_matchup(self):
        if self.matchup_outcome in self.matchup_dict:
            self.matchup_dict[self.matchup_outcome] += 1
        else:
            self.matchup_dict[self.matchup_outcome] = 1
        if(self.matchup_dict[self.matchup_outcome] > self.matchup_dict[self.most_common_matchup]):
            self.most_common_matchup = self.matchup_outcome


def solution(george: str, john: str)  -> str:
    # refactoring ca be added to assert correction of data
    # 
    #The input of your function consists of two strings - 
    # the symbols that George showed in the order he showed
    #  them and a second string with the
    #  set of symbols that John showed in the order he showed them.

    george_player = Player(george)
    john_player = Player(john)

    solver = RPS_solver(george_player, john_player)
    while(solver.george.has_hands()  and solver.john.has_hands()):
        solver.george.current_hand = solver.next_item(solver.george)
        solver.john.current_hand = solver.next_item(solver.john)

        #match outcome
        solver.matchup_outcome = solver.george.current_hand + solver.john.current_hand

        # count match points:
        solver.count_match_points()

        # determine current most common matchup:
        solver.get_most_common_matchup()

    result_str = f"George {solver.george.points} John {solver.john.points} {solver.most_common_matchup} {solver.matchup_dict[solver.most_common_matchup]}"
    return result_str

print(solution("OOOOOO[][]8<", "[][]8<8<8<O[]O[]"))

Question:

How can I improve this code for style (maybe even performance). Should I add validators if input is being guaranteed? Should I add comments ?

Answer 1

• You should not bake hand literal strings in your logic; instead refer to your constants or better yet an Enum
• You should try to reduce the amount of class state floating around. index and current_hand for instance are not good class members; instead they should just be local variables.
• Consider refactoring your Player class to be an iterator over its hands, and taking responsibility for parsing its hand string.
• Rather than matchup_dict, use a Counter. This will greatly simplify get_most_common_matchup.
• solution should be a method on RPS_solver.
• Rework this to be a unit test that asserts expected output.
• Do not hard-code the player names; pass them into members on the player objects.

Suggested

from collections import Counter
from enum import Enum
from typing import Iterable


class Hand(Enum):
    SCISSORS = "8<"
    PAPER = "[]"
    ROCK = "O"

    def loses_to(self, other: "Hand") -> bool:
        return LOSES_TO[self] == other


LOSES_TO = {
    Hand.SCISSORS: Hand.ROCK,
    Hand.ROCK: Hand.PAPER,
    Hand.PAPER: Hand.SCISSORS,
}


class Player:
    def __init__(self, name: str, hands: str) -> None:
        self.name, self.hands = name, hands
        self.points = 0

    def __iter__(self) -> Iterable[Hand]:
        index = 0
        while index < len(self.hands):
            for hand in Hand:
                if self.hands[index:].startswith(hand.value):
                    yield hand
                    index += len(hand.value)
                    break
            else:
                raise ValueError("Invalid hand string")

    def __str__(self) -> str:
        return f"{self.name} {self.points}"


class RPS_solver:
    def __init__(self, george: Player, john: Player) -> None:
        self.george = george
        self.john = john
        self.matchup_counts = Counter()

    def count_match_points(self, george_hand: Hand, john_hand: Hand) -> None:
        if george_hand == john_hand:
            self.george.points += 1
            self.john.points += 1
        elif george_hand.loses_to(john_hand):
            self.john.points += 2
        else:
            # assuming all input data is correct
            self.george.points += 2

    def update_counts(self, george_hand: Hand, john_hand: Hand) -> None:
        self.matchup_counts[george_hand, john_hand] += 1

    def solve(self) -> str:
        # refactoring can be added to assert correction of data
        #
        # The input of your function consists of two strings -
        # the symbols that George showed in the order he showed
        #  them and a second string with the
        #  set of symbols that John showed in the order he showed them.

        for george_hand, john_hand in zip(self.george, self.john):
            self.count_match_points(george_hand, john_hand)
            self.update_counts(george_hand, john_hand)

        ((george_common, john_common), count), = self.matchup_counts.most_common(1)

        return (
            f"{self.george} {self.john} "
            f"{george_common.value}{john_common.value} {count}"
        )


def test() -> None:
    solver = RPS_solver(
        Player("George", "OOOOOO[][]8<"),
        Player("John", "[][]8<8<8<O[]O[]"),
    )
    assert solver.solve() == "George 12 John 6 O8< 3"


if __name__ == "__main__":
    test()