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

Trying to grasp OOP Rock, Paper, Scissors program

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

Problem

I'm trying to wrap my head around OOP but so far I feel like I'm just moving functions into classes and calling them methods. I am trying to create a "Rock Paper Scissors" game using OOP best practices.

Yesterday I asked someone to review this code and was told that my use of OOP was akin to being handed an electric drill and using it to hammer nails so I tried again:

```
from random import randrange

class NewGame(object):
def __init__(self, num_rounds):
self.num_rounds = num_rounds
self.current_round = 0

def round(self):
human_choice = human_player.choose()
comp_choice = comp_player.choose()
decide_winner()
game.current_round += 1

class Player(object):
def __init__(self):
self.score = 0
self.choice = None

def choose(self):
self.choice = int(raw_input("Rock [1] Paper [2] Scissors [3]"))
return self.choice

def win_round(self):
self.score += 1

def win_game(self):
print("YOU WON!")

class ComputerPlayer(Player):

def choose(self):
self.comp_choice = randrange(1, 4)
return self.comp_choice

def win_game(self):
print("THE COMPUTER WON!")

def who_won(score1, score2):
human_player_score = score1
comp_player_score = score2
if human_player_score > comp_player_score:
human_player.win_game()
if human_player_score < comp_player_score:
comp_player.win_game()
if human_player_score == comp_player_score:
print ("IT WAS A TIE!")

def decide_winner():

if human_player.choice == comp_player.comp_choice:
print("It was a tie.")
elif human_player.choice == 3 and comp_player.comp_choice == 2:
print("The computer has won. Scissors cut paper.")
comp_player.win_round()
elif human_player.choice == 2 and comp_player.comp_choice == 3:
print ("You won. Scissors cut paper.")
human_player.win_round()
elif human_player.choice == 2 and c

Solution

Your Player and ComputerPlayer are good, with a very sensible use of inheritance. The NewGame is a bit of a mess, though; you have a tight coupling between the class and some sort-of-separate functions, with global variables (traditionally a bad sign), and the players aren't really related to the game.

Here's one alternative:

from random import randrange

class Game(object):

    OUTCOMES = {(1, 1): 0, (2, 2): 0, (3, 3): 0, # tie
                (1, 2): -1, (2, 3): -1, (3, 1): -1, # p1 lose
                (2, 1): 1, (3, 2): 1, (1, 3): 1} # p1 win

    def __init__(self, num_rounds):
        self.num_rounds = num_rounds
        self.human_player = Player()
        self.comp_player = ComputerPlayer()

    def play(self):
        for _ in self.num_rounds:
            outcome = self.OUTCOMES((self.human_player.choose(),
                                     self.comp_player.choose()))
            if outcome == 1:
                self.human_player.score += 1
            elif outcome == -1:
                self.comp_player.score += 1
        if self.human_player.score > self.comp_player.score:
            print "You won."
        elif self.human_player.score < self.comp_player.score:
            print "The computer won."
        else:
            print "It was a tie."

class Player(object):

    def __init__(self):
        self.score = 0

    def choose(self):
        while True:
            try:
                i = int(raw_input("Rock [1] Paper [2] Scissors [3]"))
            except ValueError:
                print "Must be an integer."
            else:
                if i in range(1, 4):
                    return i
                print "Must be between one and three." 

class ComputerPlayer(Player):

    def choose(self):
        return randrange(1, 4)

if __name__ == '__main__':
    while True:
        try:
            num_rounds = int(raw_input("How many rounds? "))
        except ValueError:
            print "Must be an integer."
        else:
            break
    Game(num_rounds).play()


Note the following:

  • No more global - we play the Game directly and both Players belong to it, so everything is accessible within the instance methods;



  • All Games use the same OUTCOMES, so I've made that a class attribute, and the dictionary lookup makes working out who won much easier;



  • Input validation for the number of rounds and the human player's choice, so the program doesn't crash if they type 'foo';



  • No need for special methods to increment a player's score, you can access it directly; and



  • All the logic for playing is now in the Game itself.



A few places for further improvement (so you can have some of the fun):

  • The choose instance methods don't use any class or instance attributes, so perhaps they should be static methods instead;



  • The Players now seem too closely coupled to the Game - if choose took a choices argument (e.g. {1: 'rock', 2: 'paper', 3: 'scissors'}, which could be another Game class attribute) the coupling would be reduced; and



  • It doesn't tell you much about the outcome now - if OUTCOMES included e.g. {(1, 2): (-1, "paper wraps rock"), ...} you could add more feedback to the user.



Once those changes are made, you should be able to define e.g.:

class HarderGame(Game):

    CHOICES = {1: 'rock', 2: 'paper', 3: 'scissors'
               4: 'lizard', 5: 'Spock'}

    OUTCOMES = {(4, 5): (1, "lizard poisons Spock"), ...}


with everything else inherited (if you don't know the outcomes, see e.g. Wikipedia), and play it exactly the same:

HarderGame(num_rounds).play()

Code Snippets

from random import randrange

class Game(object):

    OUTCOMES = {(1, 1): 0, (2, 2): 0, (3, 3): 0, # tie
                (1, 2): -1, (2, 3): -1, (3, 1): -1, # p1 lose
                (2, 1): 1, (3, 2): 1, (1, 3): 1} # p1 win

    def __init__(self, num_rounds):
        self.num_rounds = num_rounds
        self.human_player = Player()
        self.comp_player = ComputerPlayer()

    def play(self):
        for _ in self.num_rounds:
            outcome = self.OUTCOMES((self.human_player.choose(),
                                     self.comp_player.choose()))
            if outcome == 1:
                self.human_player.score += 1
            elif outcome == -1:
                self.comp_player.score += 1
        if self.human_player.score > self.comp_player.score:
            print "You won."
        elif self.human_player.score < self.comp_player.score:
            print "The computer won."
        else:
            print "It was a tie."


class Player(object):

    def __init__(self):
        self.score = 0

    def choose(self):
        while True:
            try:
                i = int(raw_input("Rock [1] Paper [2] Scissors [3]"))
            except ValueError:
                print "Must be an integer."
            else:
                if i in range(1, 4):
                    return i
                print "Must be between one and three." 


class ComputerPlayer(Player):

    def choose(self):
        return randrange(1, 4)


if __name__ == '__main__':
    while True:
        try:
            num_rounds = int(raw_input("How many rounds? "))
        except ValueError:
            print "Must be an integer."
        else:
            break
    Game(num_rounds).play()
class HarderGame(Game):

    CHOICES = {1: 'rock', 2: 'paper', 3: 'scissors'
               4: 'lizard', 5: 'Spock'}

    OUTCOMES = {(4, 5): (1, "lizard poisons Spock"), ...}
HarderGame(num_rounds).play()

Context

StackExchange Code Review Q#59164, answer score: 8

Revisions (0)

No revisions yet.