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

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 2

I am not sure the game is abstracted well enough. A move is an event which takes two players. In other words, a move is not a player, and player is not a move. What do you think about this:

# notice that the element k+1 defeats element k
THROWS = ['paper', 'scissors', 'rock']

class Player(object):
    def __init__(self, name, throws):
    # name the player
    self.name = name
    # the throws are contained a priori
    self.throws = throws

    def throw(self):
    # a throw uses (and removes) the first element of the throws
    # list
    return self.throw_value(self.throws.pop(0))

    def throw_value(self, what):
    if what in [0,1,2]:
        # if the throw is a legal int, return it
        return what
    if what in THROWS:
        # if the throw is a legal str, return the
        # corresponding int
        return THROWS.index(what)
    # if none of the above, raise error
    raise ValueError('invalid throw')

class Game(object):
    def __init__(self, player_1, player_2):
    # a game has two players
    self.player_1 = player_1
    self.player_2 = player_2

    def go(self, throws=3):
    # a "go" of the game throws three times
    for _ in range(throws):
        print self.throw()

    def throw(self):
    # a throw contains the rules for winning
    value_1 = self.player_1.throw()
    value_2 = self.player_2.throw()

    if value_1 == value_2:
        return 'draw'

    if value_1 > value_2:
        return self.player_1.name

    return self.player_2.name

if __name__ == "__main__":
    juan = Player("Juan", ['rock', 0, 'scissors'])

    jose = Player("Jose", [1, 'scissors', 2])

    game = Game(juan, jose)

    game.go()

Answer 3

mv = {"Scissor":0, "Rock":1, "Paper":2}
def winner(m1, m2):
  result = "Tie" if m1 == m2 else max(m1, m2) if abs(m1 - m2) != (len(mv) - 1) else min(m1, m2)
  return mv.keys()[mv.values().index(result)] if result in mv.values() else result

I wrote this to prove the concept: with 5 lines and almost no object orientation you can achieve the stated result, paper; rock; scissor.

A dictionary of numbers/strings. If you pass the numbers in, your result will be the name of the winning string. The validity of the win is sequential (a < b < c < a) so you can simply do a distance check to determine the need to override the sequence. I have added "Tie" as that is an obvious case but really constructing the game with players and all is trivial with this method. Now if you want to play Paper, Rock, Scissors, Lizard, Spock we would need to refactor.

Answer 4

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.