Edited simple Blackjack game in Python 3.4

Question

I'm a beginning Python programmer, and earlier I made a simple Blackjack game in Python. The original version (which can be found here) got a lot of great feedback, so I rewrote the entire program to reflect the feedback I received. Again, any tips are appreciated.

import random
import itertools

# generates the deck and card values
SUIT = 'scdh'
RANK = '23456789TJQKA'
DECK = tuple(''.join(card) for card in itertools.product(RANK, SUIT))
VAL = []
for a in range(9):
    VAL += [a+2] * 4
for _ in range(3):
    VAL += [10] * 4
VAL += [1] * 4
DECKVAL = dict(zip(DECK, VAL))
counter = 0


def shuffle():
    # shuffles the deck into random order
    global s_deck
    s_deck = random.sample(DECK, 52)


class Hand:

    global counter, s_deck

    def __init__(self):
        # deals 2 cards
        # sets up variables hand_sum and init_sum for totaling
        global counter
        self.cards = list(s_deck[counter:counter + 2])
        counter += 2
        self.hand_sum = 0
        self.init_sum = 0

    def sum_hand(self):
        # totals the hand
        self.hand_sum = 0
        self.init_sum = 0
        for b in range(len(self.cards)):
            self.init_sum += DECKVAL[self.cards[b]]
        if ('As' in self.cards or 'Ac' in self.cards or 'Ad' in self.cards or 'Ah' in self.cards)\
                and self.cards and self.init_sum + 10 <= 21:    # special ace rule conditions
            self.hand_sum = self.init_sum + 10
        else:
            self.hand_sum = self.init_sum

    def draw(self):
        # draws a card from the deck
        global counter
        self.cards += list(s_deck[counter:counter + 1])
        counter += 1

    def hit_or_stay(self, dealer):
        # asks the player if they want to hit or stay
        self.bust(dealer)
        print("\n\nDealer's Hand:", dealer.cards[0], "--")
        print("Your hand is:", self.cards, "   Your sum is:", self.hand_sum, "\n")
        choice = input("Hit or Stay? ").lower()
        if choice == 'hit':
            self.draw()
            self.hit_or_stay(dealer)
        elif choice == 'stay':
            self.total(dealer)
        else:
            print("Please enter hit or stay")
            self.hit_or_stay(dealer)

    def total(self, dealer):
        # determines winner
        if self.hand_sum > dealer.hand_sum:
            print("You won the hand")
        elif self.hand_sum < dealer.hand_sum:
            if dealer.hand_sum <= 21:
                print("You lost the hand")
            else:
                print("Dealer busted")
        else:
            print("You tied")
        print("Dealer's hand:", dealer.cards, "   Dealer's sum:", dealer.hand_sum)
        print("Your hand:", self.cards, "Your sum:", self.hand_sum)
        print("\n*******************\n")

    def bust(self, dealer):
        # checks for player busting
        self.sum_hand()
        if self.hand_sum > 21:
            print("YOU BUSTED\n")
            self.hand_sum = 0
            self.total(dealer)
        else:
            pass


class Dealer(Hand):

    def __init__(self):
        Hand.__init__(self)
        self.logic()

    def logic(self):
        # makes sure the dealer hits "soft 17"
        self.sum_hand()
        while self.hand_sum < 17:
            self.draw()
            self.sum_hand()


class Game:

    def __init__(self):
        # starts the game
        x = input("Would you like to play?").lower()
        if x == "yes":
            shuffle()
            player = Hand()
            dealer = Dealer()
            player.hit_or_stay(dealer)
        else:
            pass

if __name__ == '__main__':
    game = Game()

Answer 1

1. General discussion

The organization of the classes needs work.

1. The logic for handling the deck is distributed all over the place: some of it in shuffle, some in Hand.__init__, and some in Hand.draw. It would be better to keep this together, for example in a Deck class.

2. The Hand class mixes two things: (i) a persistent data structure representing a hand of cards; (ii) gameplay logic for the player. It would be better to separate these concerns.

3. The Dealer class adds no data to the Hand class, nor does it change the behaviour of any of the methods of the Hand class. So this class is not needed.

4. The Game class does nothing: it has no data and no methods other than __init__. Again, this class is not needed.

2. Other comments

1. There's some missing game logic: normally a "blackjack" (ace plus a ten or face card) beats a 21 made from three or more cards.

2. This seems rather long-winded:

   RANK = '23456789TJQKA'
   VAL = []
   for a in range(9):
       VAL += [a+2] * 4
   for _ in range(3):
       VAL += [10] * 4
   VAL += [1] * 4
   DECKVAL = dict(zip(DECK, VAL))
   

   compared to something like this:

   RANK = 'A23456789TJQK'
   DECKVAL = {card:min(1 + RANK.index(card[0]), 10) for card in DECK}
   

3. The Hand class has a poorly designed interface. To get the value of a hand requires two steps to be carried out in sequence: (i) call sum_hand(); (ii) read the hand_sum attribute. If you omit step (i) then the hand_sum attribute might have the wrong value.

   For example, the total method omits step (i). Is this correct? It's impossible to tell by reading the method in isolation: you have to look at the logic for all the callers (and the callers of the callers, and so on) to see if they all remembered to call sum_hand().

   This is no way to structure a program! Ideally, we should be able to check the correctness of each method in isolation. There are two alternative ways to achieve this: either make hand_sum into a property that always returns the correct value (regardless of what methods you've called beforehand); or ensure that hand_sum is always up to date by re-computing it whenever a card is drawn.

   The first of these approaches is the most reliable, and it could easily be implemented using the @property decorator.

4. We could improve the code in hand_sum by:

   • writing a docstring;
   • using a local variable for init_sum (I chose v for value) instead of an attribute (since this is not needed outside this method);
   • using the built-in sum function to add up the card values;
   • using the built-in any to simplify the aces test;
   • avoiding the superfluous and self.cards condition (if the hand contains an ace then obviously it must have at least one card);
   • using a bit of math to simplify v + 10 <= 21 to v <= 11;
   • stating the ace rule instead merely alluding to it (this means that a reader can check the code is correct without having to be a blackjack expert);
   • putting the (cheap) test v <= 11 before the (less cheap) ace test, so that if the former fails we skip the latter.

   See below for the revised code.

5. The dealer logic has a comment saying "makes sure the dealer hits soft 17" but the code does not implement this. The sum_hand method always computes the hard value of the hand, and so the dealer will stay on soft 17. The solution is for a hand to have two value properties: one for the soft value and one for the hard value.

6. Drawing a card from the deck requires three steps in sequence: (i) check that counter is less than the length of the deck (otherwise there are no cards remaining to draw); (ii) read s_deck[counter]; (iii) add one to counter. Again, it would be very easy to forget one of these steps. And as far as I can see, step (i) has in fact been forgotten.

   With just two players, and the deck being shuffled after every hand, it's not possible to run out of cards. But if you wanted to shuffle less often, or add more players, then the issue would come up.

   See the Deck class below for one approach.

3. Revised code

from itertools import product
from random import sample

SUIT = 'SCDH'
RANK = 'A23456789TJQK'
DECK = tuple(''.join(card) for card in product(RANK, SUIT))
DECKVAL = {card:min(1 + RANK.index(card[0]), 10) for card in DECK}

class OutOfCards(Exception): pass

class Deck:
    """A shuffled deck of cards."""
    def __init__(self):
        self._cards = sample(DECK, len(DECK))

    def draw(self):
        """Draw one card from the deck and return it.
        Raise OutOfCards if the deck is out of cards.

        """
        if self._cards:
            return self._cards.pop()
        else:
            raise OutOfCards()

class Hand:
    """A hand of cards in blackjack."""
    def __init__(self, deck):
        self.cards = []
        for _ in range(2):
            self.draw(deck)

    def __str__(self):
        return ' '.join(self.cards)

    def draw(self, deck):
        """Draw one card from deck. Return the new card."""
        card = deck.draw()
        self.cards.append(card)
        return card

    @property
    def soft_value(self):
        """The "soft" value of the cards in the hand (counting aces as 1)."""
        return sum(DECKVAL[card] for card in self.cards)

    @property
    def value(self):
        """The "hard" value of the cards in the hand (counting aces as 11
        unless this would go bust, otherwise as 1).

        """
        v = self.soft_value
        if v <= 11 and any(card[0] == 'A' for card in self.cards):
            return v + 10
        else:
            return v

    @property
    def bust(self):
        """True if this hand is bust."""
        return self.soft_value > 21

    @property
    def blackjack(self):
        """True if this hand is a blackjack (ace plus ten or face card)."""
        return len(self.cards) == 2 and self.value == 21

def blackjack():
    """Play a hand of blackjack."""
    deck = Deck()
    hand = Hand(deck)
    dealer = Hand(deck)
    print("Dealer shows: {}".format(dealer.cards[0]))

    # Player logic
    while True:
        print("Your hand: {}".format(hand))
        if hand.bust:
            print("BUST!")
            return
        choice = input("Hit or stay? ").lower()
        if 'stay'.startswith(choice):
            break
        elif 'hit'.startswith(choice):
            card = hand.draw(deck)
            print("You drew: {}".format(card))

    # Dealer logic ("soft 17" variant)
    while True:
        print("Dealer's hand: {}".format(dealer))
        if dealer.soft_value >= 17:
            break
        card = dealer.draw(deck)
        print("Dealer drew: {}".format(card))
    if dealer.bust:
        print("Dealer is bust.")
    elif dealer.blackjack and not hand.blackjack:
        print("Dealer's blackjack beats your {}".format(hand.value))
    elif hand.blackjack and not dealer.blackjack:
        print("Your blackjack beats dealer's {}".format(dealer.value))
    elif dealer.value > hand.value:
        print("Dealer's {} beats your {}".format(dealer.value, hand.value))
    elif hand.value > dealer.value:
        print("Your {} beats dealer's {}".format(hand.value, dealer.value))
    else:
        print("Push: {} each".format(hand.value))

Answer 2

• My first tip would be to get rid of the Game class. It serves no value. Rather, it should be a method, appropriately named main. If you want a few tips for determining whether to write a class or not, here are a some instances where you should avoid classes:

  • If the class only contains two methods, __init__, and another.

  • If the class never refers to self. (Credit to @jonrsharpe)

• Secondly, those comments underneath your function declarations should be docstrings. A typical function docstring would look something like this,

  def my_func(args):
      """
      A detailed description of the function
      and its arguments.
      """
  

• The below statement in Game.__init__ is not needed, and does nothing. It can be removed.

  else:
      pass
  

• You have some odd naming in places. For example, logic could be renamed to check_for_soft_17 or something along those lines, and total could be renamed to determine_winner.

• The two for loops underneath the VAL = [] could be shortened to generator expressions. Here's how that would be done.

  VAL = [[a + 2] * 4 for a in range(9)] + [[10] * 4 for _ in range(3)]
  

• Preferably, the variable a in the above example should also be renamed to something a little more descriptive, and you should apply some of those "magic numbers" to variables as well.

• As mentioned by @Josay, you're using globals in a lot of your functions. Rather than assigning a value to a global, try returning a value instead. It makes things a whole lot easier to manage.

Anyways, that's about all I can think of. If there's anything else that you want me to cover, just mention it in the comments, and I'll see what I can do. I hope this helps!