How can I make this code Pythonic

Question

So I have this code for an object. That object being a move you can make in a game of rock papers scissor. Now, the object needs to be both an integer (for matching a protocol) and a string for convenience of writing and viewing.

class Move:
    def __init__(self, setMove):
        self.numToName = {0:"rock", 1:"paper",2:"scissors"} 
        self.nameToNum = dict(reversed(pairing) for pairing in self.numToName.items())
        if setMove in self.numToName.keys():
            self.mMove=setMove
        else:
            self.mMove=self.nameToNum.get(setMove) #make it to a number

    def defeats(self):
        return Move((self.mMove-1)%3)
    def losesTo(self):
        return Move((self.mMove+1)%3)
    def tiesWith(self):
        return self

    #Operator overloading
    def __eq__(A,B):
        return A.mMove==B.mMove
    def __gt__(A,B):
        return A.defeats(B)
    def __lt__(A,B):
        return A.losesTo(B)
    def __ge__(A,B):
        return A>B or A==B
    def __le__(A,B):
        return A<B or A==B

    def __str__(self):
        return self.numToName.get(self.mMove);

    def __int__(self):
        return self.mMove;

Now I'm kinda new to python, coming from a C and Java background. A big thing in python is that there is only one correct way to do something. Another thing is not worrying about type. I'm pretty explicitly worrying about type here.

So I'm not sure what the correct way to handle these objects is. At the moment I have an object which an be one of any 3 types (or more but I'm not sure what that would do) Maybe instead I should be used the objects of different classes? and make them singletons? Also my object are currently modifiable after creation, which is a bad thing in my mind.

So is this code Pythonic, and how can i make it more elegant? (I figure this is a good example to use, to help me work out what makes good python code. Sorry if it seems a bit open ended)

Answer 1

To me, the concept of code being "pythonic" really comes down to the idea that once you understand what problem you're trying to solve, the code almost writes itself. In this case, without worrying about the deeper abstractions of players, games, throws, etc., you have the following problem: there are a certain number of types of moves, each with a name, with set rules for which moves beat which other moves, and you need to find a way to define moves and figure out which move wins in a comparison.

When I read your code, I don't immediately see this problem, I see a lot of extra thought that went into the code itself, finding type representations, doing arithmetic tricks, and generally forcing the problem into a code framework, rather than the other way around. So I'd suggest something like:

class Move:
  TYPES = ['rock', 'paper', 'scissors']
  BEATS = {
    'rock': ['scissors'],
    'paper': ['rock'],
    'scissors': ['paper']
  }

  def __init__(self, type):
    if type not in self.TYPES:
      raise Exception("Invalid move type")
    self.type = type

  def __str__(self):
    return self.type

  def __cmp__(self, other):
    if other.type in self.BEATS[self.type]:
      return 1
    elif self.type in self.BEATS[other.type]:
      return -1
    else:
      return 0

And you're done. You can throw in all the other accessors, etc. but it's really just icing, the core problem is solved and the code is readable, flexible, easy to extend, etc. That's really what I think "pythonic" means.

Answer 2

Well, you have only three possible moves, right? Why not just represent them as strings? It seems like the only reason you have the numbers at all is to implement the comparisons (i.e. which defeats which) with some "clever" math, but honestly I don't think that's worth it. All you really need is a function to determine which one is the winner in each possible comparison:

def winner(move0, move1):
    if move0 == move1:
        return None
    elif (move0 == 'rock' and move1 == 'scissors') or \
         (...paper vs. rock...) or \
         (...scissors vs. paper...):
        return 0
    else:
        return 1

I just made up the return values None, 0, and 1 as an example, you could use whatever is appropriate for your situation.

"Simple is better than complex," The Zen of Python line 3 ;-)

Answer 3

Here is a short version that verbalizes the result.

def winner(p1, p2):
    actors = ['Paper', 'Scissors', 'Rock']
    verbs = {'RoSc':'breaks', 'ScPa':'cut', 'PaRo':'covers'}
    p1, p2 = actors.index(p1), actors.index(p2)
    winner, looser = ((p1, p2), (p2, p1))[(1,0,1)[p1 - p2]]
    return ' '.join([actors[winner],
                     verbs.get(actors[winner][0:2] + actors[looser][0:2],
                               'ties'),
                     actors[looser]])

The benefit of this structure is evident when expanded to cover Rock, Paper, Scissors, Lizard, Spock

def winner(p1, p2):
    actors = ['Paper', 'Scissors', 'Spock', 'Lizard', 'Rock']
    verbs = {'RoLi':'crushes', 'RoSc':'breaks', 'LiSp':'poisons',
             'LiPa':'eats', 'SpSc':'smashes', 'SpRo':'vaporizes', 
             'ScPa':'cut', 'ScLi':'decapitate', 'PaRo':'covers', 
             'PaSp':'disproves'}
    p1, p2 = actors.index(p1), actors.index(p2)
    winner, looser = ((p1, p2), (p2, p1))[(1,0,1,0,1)[p1 - p2]]
    return ' '.join([actors[winner],
                     verbs.get(actors[winner][0:2] + actors[looser][0:2],
                               'ties'),
                     actors[looser]])

>>> winner("Rock", "Scissors")
'Rock breaks Scissors'
>>> winner("Rock", "Spock")
'Spock vaporizes Rock'
>>> winner("Spock", "Paper")
'Paper disproves Spock'
>>> winner("Lizard", "Scissors")
'Scissors decapitate Lizard'
>>> winner("Paper", "Paper")
'Paper ties Paper'