HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Directly accessing class attributes in Rock Paper Scissors

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
paperscissorsattributesrockdirectlyclassaccessing

Problem

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

Solution

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()

Code Snippets

Rock -> Scissors -> Paper -> Rock
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.
def __call__(self):
    return self.hand
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()

Context

StackExchange Code Review Q#40784, answer score: 2

Revisions (0)

No revisions yet.