Shuffling the Deck
Your comment # The deck is shuffled only once, when the deck is first built is wrong. You are shuffling the deck 52 times; once after before each card is added:
deck = []
for value in range(13):
suit_list = ['Hearts', 'Diamonds', 'Clubs', 'Spades']
for suit in suit_list:
...
random.shuffle(deck) # This line is executed 13 * 4 times!
deck.append((name, face, suit, value))
return deck
Additionally, you are grabbing cards from a random location in the middle of the deck, so it doesn't matter if you shuffled the deck or not:
selected_card = (random.choice(deck))
...
deck.remove(selected_card)
You should build the complete deck, and then shuffle it once:
deck = []
for value in range(13):
suit_list = ['Hearts', 'Diamonds', 'Clubs', 'Spades']
for suit in suit_list:
...
deck.append((name, face, suit, value))
random.shuffle(deck)
return deck
And then, deal from the top of the deck:
selected_card = deck.pop(0)
Suits
for value in range(13):
value += 1
suit_list = ['Hearts', 'Diamonds', 'Clubs', 'Spades']
for suit in suit_list:
The suit_list doesn't change. It doesn't need to be recreated for each of the 13 card ranks. You should move it out of the inner loop.
There it is still a local variable. Code which wants to use your deck of cards might want/need to know the what all the suits in your deck are. Is it a normal playing card deck, or a deck of Tarot cards? Is the spade suit represented as "Spades", "SPADES", "S", "♠" or "♤"?
An enum would be a better entity to use as a card suit:
from enum import Enum
Suit = Enum('Suit', 'Clubs, Diamonds, Hearts, Spades')
Then a program could safely refer to the spade suit as Suit.Spades, and iinstead of:
suit_list = ['Hearts', 'Diamonds', 'Clubs', 'Spades']
for suit in suit_list:
You'd simply iterate over the Suit enum:
for suit in Suit:
You can still use suit.name to get a nice string for the suit.
range(start, end, step)
Instead of writing:
for value in range(13):
value += 1
...
use the fact that a range can start at any value, so you don't need to add 1 to the card's rank at each iteration; just iterate over a different range:
for value in range(1, 13+1):
...
Each tuple represents a card ...
Ok ... lemme think. The card[0] is the card's rank, and card[1] is the card's suit? Or do have have that backwards ; card[0] is the suit and card[1] is the rank?
With a tuple, it is easy to forget which member is stored in which field. It is way easier to use a named tuple:
from collections import namedtuple
Card = nametuple('Card', 'name, face, suit, value')
Then we wouldn't have to remember; card.rank is the the card's rank, and card.suit is the card's suit.
The only downside is ... uhh ... no, sorry, there are no downsides. A namedtuple is just as efficient time and space wise. You'd create the cards like this:
deck.append(Card(name, face, suit, value))
And instead of
print('You drew %s.' % (selected_card[0]))
you'd write:
print('You drew %s.' % selected_card.name)
Formatting
The inner parenthesis are unnecessary here:
print('You drew %s.' % (selected_card[0]))
They take the value "the Ace of Spaces", and ... return that string unaltered. So we end up with the expression str % str, and since the first string contains only one %s code, the argument is directly used.
if you had used (selected_card[0], ), that would have constructed a tuple of 1 values, a string, which could also be applied to that format string. Without the trailing comma, you don't have a tuple.
It is easier to use f-strings. There, the format arguments are placed directly in the format codes, instead of at the end where they have to be match up by positions. Instead of:
name = 'the %s of %s' % (face, suit)
you could write:
name = f'the {face} of {suit}'
which is slightly more compact, and much easier to tell where the values are going in the resulting string.
Hand size
def draw_hand(hand_size):
player_hand = []
for draw in range(int(hand_size)):
Wait ... what is that int(...) doing there? What are you expecting to pass to draw_hand, if not an integer? Why would you allow it to be a string?
It should be the caller's responsibility for any string to integer conversions. This function should only expect an integer for hand_size.
Improved code
Using NamedTuple instead of namedtuple, a Rank enumeration, and reducing Card to just a tuple of rank and suit, with str(card) corresponding to the card's name, and card.rank.value for the card's value ... along with some other structural modifications:
from random import shuffle
from enum import Enum
from typing import NamedTuple, List
from itertools import product
Suit = Enum('Suit', 'Clubs, Diamonds, Hearts, Spades')
Rank = Enum('Rank', 'Ace, 2, 3, 4, 5, 6, 7, 8, 9, 10, Jack, Queen, King')
class Card(NamedTuple):
rank: Rank
suit: Suit
def __str__(self):
return f"{self.rank.name} of {self.suit.name}"
def build_deck() -> List[Card]:
deck = [Card(rank, suit) for rank, suit in product(Rank, Suit)]
shuffle(deck)
return deck
def draw_hand(hand_size: int) -> List[Card]:
return [deck.pop(0) for _ in range(hand_size)]
def print_hand(hand: List[Card]) -> None:
print("You drew:", ", ".join(str(card) for card in hand))
if __name__ == '__main__':
deck = build_deck()
n = int(input(f"How many cards? There are {len(deck)} cards in the deck: "))
hand = draw_hand(n)
print_hand(hand)
Result:
How many cards? There are 52 cards in the deck: 5
You drew: 2 of Hearts, 9 of Clubs, 5 of Hearts, 7 of Hearts, Queen of Clubs
