Directly accessing class attributes in Rock Paper Scissors

Question

I am reading a beginning Python programming book and in it the author says that you want to avoid accessing a class' attributes directly, but instead to create methods that return attribute values. However, I am finding there are times this seems a little redundant and I am wondering if am just going about this in the wrong way.

An example of this is in the battle function where I access player1.hand directly. Is this a bad idea? Also, does everything else look okay in this or are there any bad practices I should be nipping in the bud?

#a game of rock paper scissors for two computers
#imports
import random
class Hand(object):
 """A hand with which to rock paper scissor"""
 HANDS = ("rock", "paper", "scissors")
 def __init__(self, name, wins = 0, hand = None):
 self.name = name
 self.wins = wins
 self.hand = hand
 def choose_hand(self):
 """Chooses hand for player"""
 self.hand = random.choice(self.HANDS)
 print self.name + " picks " + self.hand + "."
 def increase_wins(self):
 """Increases players wins and declares them this rounds winner"""
 self.wins += 1
 print self.name + " wins."
 def total_wins(self):
 """shows total wins of all rounds played"""
 print self.name + " won " + str(self.wins) + " rounds."
def battle(player1, player2):
 """plays one round of rock paper scissor"""
 #players choose their hands
 player1.choose_hand()
 player2.choose_hand()
 #see who wins the battle
 if player1.hand == "rock" and player2.hand == "scissors":
 player1.increase_wins()
 elif player1.hand == "paper" and player2.hand == "rock":
 player1.increase_wins()
 elif player1.hand == "scissors" and player2.hand == "paper":
 player1.increase_wins()
 elif player1.hand == player2.hand:
 print "It's a tie."
 else:
 player2.increase_wins()
def main():
 player1 = Hand("HAL")
 player2 = Hand("WOPR")
 rounds = 10
 for i in range(rounds):
 print "\nRound " + str(i + 1) + ":"
 battle(player1, player2)
 print "\nFinal Score:"
 player1.total_wins()
 player2.total_wins()
 if player1.wins > player2.wins:
 print player1.name + " wins."
 elif player1.wins == player2.wins:
 print "It's a tie."
 else:
 print player2.name + " wins."
#play the game
main()

Answer 1

There's nothing wrong with using attributes in Python. The author of your book is mistaken on this point. He or she must have half-remembered a rule about not using attributes, but doesn't know (or has forgotten) that this rule applies:

1. to programming languages like Java, in which attribute (member) access cannot later be changed to use a method call, without rewriting all the callers; and

2. when you are publishing an API for use by other programmers, so that there will be a body of code that depends on the published details of the API, which needs to be kept compatible.

But point 1 does not apply to Python: if at some later time you realise that you need to replace an attribute with a method, you can do so easily via the @property decorator. And point 2 does not apply to the code in the question: there is no public or published API here. If you later want to change Player.hand to Player.hand(), it's an easy change to make, because all the code is yours.

Python uses attributes in many places in the standard library, for example, subprocess.Popen.returncode, tempfile.NamedTemporaryFile.name, datetime.date.year, threading.Thread.daemon. (You'll see that in the last case, the old getter/setter API has been deprecated in favour of the attribute, because attribute access is simpler and clearer.)

It's important to know, when applying a rule like "avoid attributes and use methods instead", the reasons behind the rule, and the situations where it is and isn't appropriate. Don't apply rules blindly!

(A note on terminology: you talk about "class attributes" in the question: but properly speaking, the hand attribute belongs to instances of the class, not to the class itself: that is, Player(...).hand is OK but Player.hand is not.)

• Michael Urman: Strongly agreed. Other languages bake in the kind of access in a way that differs for what python calls attributes and properties. But since in python the mechanism is the same in the calling code, there's no pain when you have to change an attribute to a property or vice versa.

Answer 2

The point of using classes is to encapsulate behavior and data. Exposing hand means if you change the internal representation -- say from a string to a class -- you then have to update any code using class Hand. Also, you imply that foo.hand = thing is valid.

Consider instead having a method which tests if the Hand defeats a supplied option:

def defeats(self, value):
 if self.hand == value:
 return 0 # aka tie
 elif self.hand == "rock" and value == "paper":
 return -1 # lose
 elif self.hand == "paper" and value == "rock":
 return 1 # win

• Gareth Rees: This answer might be good advice in some languages, but in Python if you need to change the implementation of an attribute you can replace it with a property.
• Sean Perry: True, but it does not change the point of OO and classes. Properties are nice and exposed data makes for rapid prototyping. But when you are designing an API it makes sense to make things that are easy, obvious, and maintainable. Might as well get into the habit so doing the "right" thing just becomes how you code.
• Gareth Rees: This begs the question (how do you know that avoiding attributes is the "right" thing?). See my answer.
• Michael Urman: While player1.defeats(player2.hand) fails to prevent direct access to hand, I like where this thought leads. I could totally see making hand an instance of a type that lets you compare player1.hand < player2.hand meaningfully, or even player1.hand.defeats(player2.hand).

Answer 3

As has been pointed out, overriding the __call__ method is not the best way to improve the readability of your code.

Using attributes is not, in itself, a problem.

What you may want to consider is a trick you can play with Rock/Paper/Scissors.

If you create an array in the order:

Rock -> Scissors -> Paper -> Rock

Then, if you have two hands, and they select a random 'hand' from the three options Rock, Scissors, Paper.... then, you can find the first index of the first hand's value in the size-4 array above.... then if the next value is the opponent's hand, you win. If it is the same as your hand, it's a draw. Otherwise it's a loss.

winconditions = ['Rock', 'Scissors', 'Paper', 'Rock']
p1index = winconditions.index(player1.hand)
p2index = winconditions.index(player2.hand)
if p1index == p2index:
 # draw
elif p1index + 1 == p2index:
 # p1 wins!
else
 # p2 wins.

---

(削除) Since this is probably the most common method to call when accessing the player, it may make sense to make the hand the result of the __call__ function ..... and it would make the code read pretty nicely....

def __call__(self):
 return self.hand

and then you can use it like:

def battle(player1, player2):
 """plays one round of rock paper scissor"""
 #players choose their hands
 player1.choose_hand()
 player2.choose_hand()
 #see who wins the battle
 if player1() == "rock" and player2() == "scissors":
 player1.increase_wins()
 elif player1() == "paper" and player2() == "rock":
 player1.increase_wins()
 elif player1() == "scissors" and player2() == "paper":
 player1.increase_wins()
 elif player1() == player2():
 print "It's a tie."
 else:
 player2.increase_wins()

(削除ここまで)

• Michael Urman: I disagree; making it look like a method call implies certain things about it that don't apply here. I can't read your sample without thinking it's making tons of player1 and player2 objects. Now, on the other hand, if you overrode __eq__ that might make sense.
• rolfl: @MichaelUrman - you are right, it is abuse of the __call__ to use it this way.... hmmm, and I don't particularly like the __eq__ option either.... since the players are not equal, but their hands are..... still stewing.