Skip to main content
Code Review

Return to Revisions

2 of 2
typo
AJNeufeld
  • 35.3k
  • 5
  • 41
  • 103

Python 3 Classes

class Card(object): is deprecated Python 2 syntax. With Python 3, the (object) base class is implied and just unnecessary clutter. Just write class Card:.

One Source of Truth

Each Card has a dictionary converting the card rank to a value. With 52 cards in a deck, you'll have 52 identical dictionaries. With 5 decks in the shoe, you'll have 260 identical dictionaries!

Additionally, each Deck has a tuple of ranks. 5 decks yields 5 identical tuples of card ranks.

(Again, additionally, a Shoe inherits from Deck (brokenly, more later), so even more copies???)

You should specify the card ranks exactly once in your program, in one global structure. An Enum would be a good choice for this:

from enum import Enum
class Rank(Enum):
 TWO = ('2', 2)
 THREE = ('3', 3)
 FOUR = ('4', 4)
 FIVE = ('5', 5)
 SIX = ('6', 6)
 SEVEN = ('7', 7)
 EIGHT = ('8', 8)
 NINE = ('9', 9)
 TEN = ('10', 10)
 JACK = ('J', 10)
 QUEEN = ('Q', 10)
 KING = ('K', 10)
 ACE = ('A', 11)
 @property
 def symbol(self):
 return self.value[0]
 @property
 def points(self):
 return self.value[1]

Similarly, you could use an Enum for suits:

class Suit(Enum):
 CLUBS = ('Clubs', '♣')
 DIAMONDS = ('Diamonds', '♦')
 HEART = ('Hearts', '♥')
 SPADES = ('Spades', '♠')
 @property
 def suit_name(self):
 return self.value[0]
 @property
 def symbol(self):
 return self.value[1]

Card

A card is pair of values: rank & suit. But more than that, it has a string which describes it, like "Queen of Clubs ♣". You use f"{rank} of {suit}" in two different places to convert the a card (actually a tuple) into a suitable string. Since you have a Card class, it should be able to convert itself into a suitable string.

A card has a point value, which (in the case of blackjack) is determined solely by the card's rank. But we shouldn't ask the card for its rank and then convert that into a point value, rather we should just ask the card for its point value, and let it determine its value from its rank & suit.

Cards are immutable. If you have a Card, you shouldn't be able to change it to another suit or rank with a statement like card.rank = 'A'. We can use a dataclass to make our Card object resistant to malicious changes:

from dataclasses import dataclass
...
@dataclass(frozen=True)
class Card:
 rank: Rank
 suit: Suit
 def point_value(self):
 return self.rank.points
 def __str__(self):
 return f"{self.rank.symbol} of {self.suit.suit_name} {self.suit.symbol}"
>>> card = Card(Rank.ACE, Suit.SPADES)
>>> print(card)
A of Spades ♠
>>> print(card.point_value())
11

Deck

class Deck(object):
 def __init__(self):
 self.deck = []

Strange. If I create a deck of cards, deck = Deck(), it doesn't have any cards it in!

The constructor for a deck of cards should actually create a deck of cards:

class Deck(object):
 def __init__(self):
 self.deck = [Card(rank, suit) for rank, suit in itertools.product(Rank, Suit)]

Note the above actually creating a list of Card objects here, not simply tuples of rank and suit values.

Your Deck class seems rather limited in its functionality. You can't return cards to the deck, you can't shuffle the remaining cards in the deck, and so on.

Shoe

class Shoe(Deck):
 def __init__(self):
 self.shoe = []

As written, a Shoe is a Deck, but without any of the functionality of a Deck. Although it inherits from Deck, since super().__init__() is never called, this inheritance is broken. Show().buildDeck() will give an AttributeError.

A Shoe is not be a Deck; don't inherit from it.

The shoe is constructed from 5 decks, sequentially! Imagine you have 5 decks, with red, yellow, green, blue, and orange backs. You're shuffling each of those decks individually, and then stacking them one on top of the other in the shoe without shuffling them together. This makes it impossible (instead of just unlikely) to draw three "Ace of Spaces ♠" sequentially from the shoe.

You should add cards from the 5 decks into the shoe, and THEN shuffle the shoe afterwards.

AJNeufeld
  • 35.3k
  • 5
  • 41
  • 103
default

AltStyle によって変換されたページ (->オリジナル) /