Rock Papers Scissors in Python

Question

I am a newbie to Python and have started object-oriented programming recently. I have implemented a "Rock Paper Scissors" application in OOP. I would like for you to evaluate my code, and tell me where can I improve my code and how I can better organize the functionality.

import random

class RockPaperScissors(object):        
    def setUserOption(self,val):
        self.__option = val

    def getValueFromList(self,option):
        l = ['rock','scissors','paper']
        return l[option]

    def __getRandomValue(self,max):
        val = random.randint(1,max)
        return self.getValueFromList(val-1)

    def __getResult(self,t):
        self.__d = {('rock','scissors'):'rock breaks scissors - User has won',('rock','paper'):'rock is captured by paper - User has won',('scissors','paper'):'Scissors cut paper - User has won',
                    ('scissors','rock'):'rock breaks scissors - Computer won',('paper','rock'):'rock is captured by paper - Computer won',('paper','scissors'):'Scissors cut paper - Computer won'}
        return self.__d[t]

    def __computeResult(self,computerOption):
        if computerOption == self.__option:
            return 'The user and computer choose the same option'
        else:
            return self.__getResult((self.__option,computerOption))

    def printResult(self):
        computerOption = self.__getRandomValue(3)
        print 'User choice: ',self.__option
        print 'Computer choice: ',computerOption
        print self.__computeResult(computerOption)

if __name__ == "__main__":
    print 'Welcome to rock paper scissors'
    print '1. Rock, 2. Paper, 3. Scissor'
    val = int(raw_input('Enter your choice Number: '))
    if val >=1 and val <= 3:
        obj = RockPaperScissors()
        obj.setUserOption(obj.getValueFromList(val-1))
        obj.printResult()
    else:
        raise ValueError('You are supposed to enter the choice given in the menu')

Answer 1

These answers are fantastic, but seem to be focusing on one side of your question. I'm going to focus on object orientation.

I reviewed your code with a couple basic concepts in mind, namely polymorphism and encapsulation.

Polymorphism

Where are your objects? A rock, paper, and scissors could all be objects, could they not? And, most importantly, they are all the same kind of object. They are all "pieces" of the game.

Here's a pseudo-example:

class Rock inherits from Element
    type = 'rock'
    def compare(Element)
        if type == 'paper'
            return 'LOSE'
        elsif type == 'scissors'
            return 'WIN'
        else
            return 'TIE'

Although this is a potential solution, my example is going to go in a different direction simply for the fact that if you wanted to add more elements then you'd have to touch the existing code (re: Chris' example of RPSLS).

Encapsulation

A great programming muscle to exercise is encapsulation. The two main areas, in this example, are to hide the user interface from the game code.

The user interface shouldn't care about the inner-workings of the game. The game shouldn't care how it's displayed. What if you wanted to change the interface to something more graphical? Right now, your code relies on the console to know about the pieces of the game and how it works.

Example

So, let's return to Chris' example again. The elements of this game are something that may change quite often. How can you handle this to make future programming easier?

One solution is to store the "data" of the game elsewhere and dynamically create objects.

The following is Python code to materialize the ideas I've written about here.

Here's code for an abstracted piece of the game, an element:

# The goal for an element is to just know when it wins, when it loses, and how to figure that out.
class Element:
    _name = ""
    _wins = {}
    _loses = {}

    def get_name(self):
        return self._name

    def add_win(self, losingElementName, action):
        self._wins[losingElementName] = action

    def add_loss(self, winningElementName, action):
        self._loses[winningElementName] = action

    def compare(self, element):
        if element.get_name() in self._wins.keys():
            return self._name + " " + self._wins[element.get_name()] + " " + element.get_name()
        elif element.get_name() in self._loses.keys():
            return None
        else:
            return "Tie"

    def __init__(self, name):
        self._name = name
        self._wins = {}
        self._loses = {}

Games have players:

# The player's only responsibility is to make a selection from a given set. Whether it be computer or human.
class Player:
    _type = ''
    _selection = ''

    def make_selection(self, arrayOfOptions):
        index = -1
        if (self._type == 'Computer'):
            index = random.randint(0, len(arrayOfOptions) - 1)
        else:
            index = int(raw_input('Enter the number of your selection: ')) - 1
        self._selection = arrayOfOptions[index]
        return self._type + ' selected ' + self._selection + '.'

    def get_selection(self):
        return self._selection

    def __init__(self, playerType):
        self._type = playerType
        self._selection = ''

Game code:

# A game should have players, game pieces, and know what to do with them.
class PlayGame:
    _player1 = Player('Human')
    _player2 = Player('Computer')

    _elements = {}

    def print_result(self, element1, element2):
        val = element1.compare(element2)
        if (val != None):
            print "YOU WIN! (" + val + ")" # win or tie
        else:
            print "You lose. (" + element2.compare(element1) + ")"


    def fire_when_ready(self):
        counter = 1
        for e in self._elements.keys():
            print str(counter) + ". " + e
            counter = counter + 1
        print ""
        print "Shoot!"
        print ""

        print self._player1.make_selection(self._elements.keys())
        print self._player2.make_selection(self._elements.keys())

        element1 = self._elements[self._player1.get_selection()]
        element2 = self._elements[self._player2.get_selection()]

        self.print_result(element1, element2)


    def load_element(self, elementName1, action, elementName2):
        winningElementObject = None
        newElementObject = None

        if (elementName1 in self._elements):
            winningElementObject = self._elements[elementName1]
            winningElementObject.add_win(elementName2, action)
        else:
            newElementObject = Element(elementName1)
            newElementObject.add_win(elementName2, action)
            self._elements[elementName1] = newElementObject

        if (elementName2 in self._elements):
            losingElementObject = self._elements[elementName2]
            losingElementObject.add_loss(elementName1, action)
        else:
            newElementObject = Element(elementName2)
            newElementObject.add_loss(elementName1, action)
            self._elements[elementName2] = newElementObject


    def __init__(self, filepath):
        # get elements from data storage
        f = open(filepath)
        for line in f:
            data = line.split(' ')
            self.load_element(data[0], data[1], data[2].replace('\n', ''))

The console code, the user interface:

if __name__ == "__main__":
    print "Welcome"
    game = PlayGame('data.txt')
    print ""
    print "Get ready!"
    print ""
    game.fire_when_ready()

And the data file, data.txt:

scissors cut paper
paper covers rock
rock crushes lizard
lizard poisons spock
spock smashes scissors
scissors decapitates lizard
lizard eats paper
paper disproves spock
spock vaporizes rock
rock crushes scissors

The goal for my answer was to show you how you can use some object oriented concepts in solving your problem. In doing so I also left some unresolved problems. One big one is that the player and game objects are still coupled to the user interface. One way to nicely resolve this would be through the use of delegates.

Answer 2

All in all, you're looking good!

Here are a few things that stand out to me:

• You're using double-underscore name mangling, when you probably don't need to. It's not the best habit to get into. Single-underscore says "I am private" without doing the horrendous name obfuscation. Consider renaming your __method() to _method().
• Inside of __getResult() you're defining __d. That means every time that method is called, that dictionary is recreated. This probably isn't what you want. Consider moving it to a class attribute
• Your method (and some variable) names are mixedCase, which breaks Python convention in accordance with the Python Style Guide (aka PEP 8). Consider changing them to lower_case_with_underscores.

Answer 3

I agree with jathanism - it does look pretty good. It's well organized and doesn't do too much.

You throw an error if an invalid option is selected. Have you considered looping until a correct value is selected?

val = 0
while val < 1 and val > 3
   val = int(raw_input('Enter your choice Number: '))

Also, you shift the value down to compensate for the 0 relative array. I'd reccomend changing the array to be artificially 1-relative:

l = ['dummy', 'rock', 'paper', 'scissors']

Alternatively you could do the shifting inside getValueFromList. Don't make the caller do it though - they shouldn't have to worry about things like that. Also, you should do some bounds checking - what happens when someone calls getValueFromList(5)? That might be a valid place to throw a ValueError.

Answer 4

Along with @jathanism's comments, I would add the following:

I would move the definition of the list of options into a class variable, e.g.:

class RockPaperScissors(object):
    OPTIONS=['rock','paper','scissors']
    ...

that way, when you want to extend the game to RockPaperScissorsLizardSpock, you can do so with a simple inheiritance:

class RockPaperScissorsLizardSpock(RockPaperScissors):
    OPTIONS=['rock','paper','scissors','lizard','spock']
    ...

The obvious extension/continuation to this comment is that __getResult would then need to be updated to be more generic in a base class.

Also, the logic for printing/making a selection could then be based on the "OPTIONS" constant. (e.g.: print ', '.join( '%s. %s' % (i,v) for i in enumerate(RockPaperScissors.OPTIONS) )). Even better, put the selection logic into a method on the class.

I try to apply these kinds of concepts to most classes, as 1) it isn't really any more work to implement, and 2) it allows for subclasses to easily override the behavior of a base class, even when you don't expect inheritance to be used.

Answer 5

Your random function:

def __getRandomValue(self,max):
    val = random.randint(1,max)
    return self.getValueFromList(val-1)

1. You only ever call this function with max = 3, since thats the only sensible argument just assume that and don't make the caller pass it
2. You pick a random value between 1 and max, and then subtract one. Better to pick a value between 0 and max-1 (random.randrange make that easy)
3. If you make the l list a class attribute, as Chris suggests, then you can use random.choice to select the option.

Answer 6

Congratulations. You've all managed to write Java in Python.

Try something like:

from random import choice
if __name__ == "__main__":
    while True:
        print "1. Rock 2. Paper 3. Scissors"
        val, comp = int(raw_input('Enter your choice Number: ')), choice(range(1,4))
        if val == comp:
            print "Draw. Try again"
        elif val == 1:
            if comp == 2:
                print "You lose"
            if comp == 3:
                print "#WINNING"
        elif val == 2:
            if comp == 3:
                print "You lose"
            if comp == 1:
                print "#WINNING"
        elif val == 3:
            if comp == 2:
                print "You lose"
            if comp == 1:
                print "#WINNING"

This, even with the ridiculously long if/else statement is better than everything proposed here.

Answer 7

It would be more pythonic to use properties (or public fields if you don't need any special getter/setter logic) instead of getter/setter methods.