Trying to grasp OOP Rock, Paper, Scissors program

Question

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 comp_player.comp_choice == 1:
        print ("You won. Paper covers rock.")
        human_player.win_round()
    elif human_player.choice == 1 and comp_player.comp_choice == 2:
        print ("You lost to the computer. Paper covers rock.")
        comp_player.win_round()
    elif human_player.choice == 1 and comp_player.comp_choice == 3:
        print ("You won. Rock smashes scissors.")
        human_player.win_round()
    elif human_player.choice == 3 and comp_player.comp_choice == 1:
        print ("You lost. Rock smashes scissors.")
        comp_player.win_round()



def game_play():
    num_rounds = raw_input("How many rounds? ")
    global game
    game = NewGame(num_rounds)
    global human_player
    global comp_player
    human_player = Player()
    comp_player = ComputerPlayer()   
    while game.current_round < int(num_rounds):
        game.round()
        print "COMPUTER: ", comp_player.score, "HUMAN: ",human_player.score
    who_won(human_player.score, comp_player.score)



if __name__ == '__main__':
    game_play()

While I feel like this is a step in the right direction I'm also pretty sure that I'm still not quite "getting it" and was hoping that you could offer suggestions on how to do better.

Also, I'm pretty sure I read that global variables should be avoided but I could not figure out how to get it to work without them. I understand that I needed to use them because of encapsulation and scope but I don't know of how to get around that without using global variables.

Answer 1

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:

1. No more global - we play the Game directly and both Players belong to it, so everything is accessible within the instance methods;
2. 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;
3. Input validation for the number of rounds and the human player's choice, so the program doesn't crash if they type 'foo';
4. No need for special methods to increment a player's score, you can access it directly; and
5. 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):

1. The choose instance methods don't use any class or instance attributes, so perhaps they should be static methods instead;
2. 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
3. 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()