I am fairly new to Python so I am trying to learn more about its conventions and ways to make my codes more efficient.
I am making a Blackjack game and so far have this class that draws, organizes, and scores a hand of cards.
class Hand (object): def __init__(self, card=, card2=, score=0): self.card = card self.card2 = card2 self.score = score def roll(self): # picks two cards and stores in card and card2 respectively as [0rank, 1suit] ranks = ["Ace", 2, 3, 4, 5, 6, 7, 8, 9, 10, "Jack", "Queen", "King"] suits = ["Hearts", "Diamonds", "Clubs", "Spades"] pick =  pick2 =  pick.append(ranks[randint(0, 12)]) pick.append(suits[randint(0, 3)]) pick2.append(ranks[randint(0, 12)]) pick2.append(suits[randint(0, 3)]) self.card = pick self.card2 = pick2 def playerScore(self): # scores player's hand, prompting ace choice value = self.card value = str(value) points = 0 if value.isdigit(): points += int(value) elif value != "Ace": points += 10 else: aceChoice = input("Play ace as 11 or 1? ") while not aceChoice.isdigit(): try: aceChoice = input("Play ace as 11 or 1? ") except TypeError: print("Invalid response.") if aceChoice == "11": points += 11 else: points += 1 self.score += points def dealerScore(self): # scores dealer's hand and automates ace choice value = self.card value = str(value) points = 0 if value.isdigit(): points += int(value) elif value != "Ace": points += 10 elif value == "Ace" and self.score >= 11: points += 1 elif value == "Ace" and self.score < 11: points += 11 self.score += points
I was wondering if I am using classes correctly, and if I should put the card list to draw from or .pop() in the class initializer, As well as the best type of data structure to store the unused cards in.
Ignore the fact that I called one method roll(). I just finished a Yahtzee game haha.
Just from an object oriented standpoint, it seems weird to have all of those methods associated with a class called
Hand. When you think of a hand in blackjack, does the hand know the dealer’s score? Probably not, the dealer knows their own score and a
player would know the scores of their own hand. Often with OOP, it makes sense to use classes and objects as they appear in the real world. A
hand class would know what the score was at the current moment in time and what cards were in that hand, but it doesn’t know the score of other hands or what the cards are in other hands.
When I think of most games, it often breaks down like this:
There is a player class.
The player has a
handobject (an instance of a class like yours) and other information related to the game the player needs to know (maybe a turn identifier, depending on the game).
There is a
board(in your game, there might be a table) class. The table would manage whose turn it is, if a player has won, and probably have a
take_turnmethod like your
rollmethod that could take in a
playerand update the table and hand accordingly.
tablemight have a deck that is an instance of a
deckclass. This class could know what cards have been played, what cards will be played, and could
dealcards to the players.
Once you start to organize your code in this way, it starts to clarify where certain datastructures should go and what responsibility a given class should have.
If you create a
deck class, you could create a method for dealing cards. In this class you could have a list of
cards. The best thing to do would be to create a class
card that encapsulates the suit and rank of the card. Then, there is 52 card objects (one for each allowed combination of rank and suit). You could add a field to your card class that indicates whether it has been played or not, ensuring when you grab a random card from the list that it is eligible for play. Alternative, you could actually remove the cards that get played from the list and when another game starts reset the deck to contain all the cards again.
I hope this is helpful. If you need more clarification just ask and I can add more details.
One thing that catches my eye:
Default list argument.
There is a bit of a footgun hiding in this method:
def __init__(self, card=, card2=, score=0): self.card = card self.card2 = card2 self.score = score
Default arguments are only created once, when the method is defined. The end result is that all Hands using default arguments will share the same lists. Observe:
1) first.card first = Hand() first.card  second = Hand() second.card.append(
I mention this not because it currently causes problems in the code (it does not;
card1 are never modified in-place), but because it is a problem you are bound to run into in the near future (or perhaps already have!).
There are a couple ways to get around this.
A good placeholder value is usually
None. The following lists are created each time the function is run:
def __init__(self, card=None, card2=None, score=0): if card is None: card =  if card2 is None: card2 =  self.card = card self.card2 = card2 self.score = score
If you immediately take a copy of any mutable objects then they are harmless:
def __init__(self, card=, card2=, score=0): self.card = list(card) # copies the input list self.card2 = list(card2) self.score = score
(“mutable” objects are those that can be modified in-place;
set are mutable.
str are immutable, and they are safe to use anywhere)
But I feel the most important alternative is probably this:
Simply require the cards to be passed in.
def __init__(self, card, card2, score=0): self.card = card self.card2 = card2 self.score = score
Generating cards and remembering (i.e. storing) cards are two separate responsibilities. It is clear (from the fact that
dealerScore both index into
self.card) that, in general, this class is expected to contain cards. Therefore, a different one should generate them.
That said, it also has the dual responsibility of both tracking a score and making decisions based upon that score. In fact, it would seem that these two are the only real reasons it needs to exist. It feels more like the class is trying to be this:
class Player: def __init__(self): self.score = 0 def playerScore(self, card): value = str(card) if value.isdigit(): # yadda yadda yadda ... def dealerScore(self, card): value = str(card) if value.isdigit(): # yadda yadda yadda ...
in which case we never even need to store the card in the class!
dealerScore duplicate some logic which can be factored out with a flag:
def addCardScore(self, card): value = str(card) if value.isdigit(): self.score += int(value) elif value == "Ace": if self.playerType == 'player': self.playerAceValue() else: self.dealerAceValue() else: self.score += 10 def playerAceValue(self): aceChoice = input("Play ace as 11 or 1? ") while not aceChoice.isdigit(): try: aceChoice = input("Play ace as 11 or 1? ") except TypeError: print("Invalid response.") if aceChoice == "11": self.score += 11 else: self.score += 1 def dealerAceValue(self): if self.score >= 11: self.score += 1 else: self.score += 11 # ... somewhere else, not necessarily in a class ... def playGame(): player = Player('player') dealer = Player('dealer') while True: playerCard = drawCard() player.addCardScore(playerCard) dealerCard = drawCard() dealer.addCardScore(dealerCard)
This code has a new issue: looking at
addCardScore, we see that two branches of the
self.score, but it is not immediately obvious that the middle branch (
elif value == 'Ace') does the same. Generally speaking, by rewriting the functions so that most of them simply return values instead of modifying members, it becomes generally easier to reason about when data is modified. Here’s an example version where only one function touches score:
def addCardScore(self, card): self.score += self.getCardScore(card) def getCardScore(self, card): value = str(card) if value.isdigit(): return int(value) elif value == "Ace": if self.playerType == 'player': return self.playerAceChoice() else: return self.dealerAceChoice() else: return 10 def playerAceChoice(self): aceChoice = input("Play ace as 11 or 1? ") while not aceChoice.isdigit(): try: aceChoice = input("Play ace as 11 or 1? ") except TypeError: print("Invalid response.") if aceChoice == "11": return += 11 else: return += 1 def dealerAceChoice(self): return 1 if self.score >= 11 else 11