5
\$\begingroup\$

I'm a beginner at coding who recently started learning Python, and I think I have grasped the basics. Now I'm working on my first "proper" project in Python.

The problem is to have the program:

  1. Generate a standard Poker deck of 52 cards (no Jokers)
  2. Shuffle said deck
  3. Deal five (5) cards to three (3) hands/"players" (can be altered when calling the 'deal' function)
  4. Analyse the three hands individually for possible Poker hands in each
    • the analysis must be able to detect at least:
      • Two pairs
      • Straight
      • Flush
      • Straight Flush
      • (no harm in detecting any of the other Poker hands as well)
  5. Print all three hands and their respective results/analyses

I've managed 1-3, but stalled at 4 and plan to continue with 4-5 later, but first I want to hear your thoughts on my code so far!

I managed to find several somewhat similar programs all over the Internet, of which I extracted many parts to my own program. I had to make a few adjustments here and there, as none of the existing programs had the exact same approach as mine, but this was a good thing in terms of my learning curve. In case you find a part of your own code in mine, consider it admiration, for I'm truly grateful for all the help and examples I've received from more seasoned coders' work.

My code:

import random

class Card(object):
    def __init__(self, suit, val):
        self.suit = suit
        self.value = val

    # Implementing build in methods so that you can print a card object
    def __unicode__(self):
        return self.show()
    def __str__(self):
        return self.show()
    def __repr__(self):
        return self.show()

    def show(self):
        if self.value == 1:
            val = "Ace"
        elif self.value == 11:
            val = "Jack"
        elif self.value == 12:
            val = "Queen"
        elif self.value == 13:
            val = "King"
        else:
            val = self.value

        return "{} of {}".format(val, self.suit)

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

    # Display all cards in the deck
    def show(self):
        for card in self.cards:
            print(card.show())

    # Generate 52 cards
    def build(self):
        self.cards = []
        for suit in ['Hearts', 'Clubs', 'Diamonds', 'Spades']:
            for val in range(1,14):
                self.cards.append(Card(suit, val))

    # Shuffle the deck
    def shuffle(self, num=1):
        random.shuffle(self.cards)

    # Deal n cards to n players
    def deal(self, n_players, n_cards):
        self.hands = [self.cards[i:n_players*n_cards:n_players] for i in range(0, n_players)]


# Test making a Card
# card = Card('Spades', 6)
# print card

# Making a Deck
myDeck = Deck()
myDeck.shuffle()
#myDeck.show() # Print the shuffled deck if needed, to show it is actually shuffled
               # and that the cards are dealt from top of the deck and not randomly

myDeck.deal(3,5) #(No. of hands, no. of cards dealt per hand)
print(*myDeck.hands, sep = '\n')

Please do let me know what you think of the code in general, and if you think there is a more efficient way to have the cards dealt so that they are also easier to analyse later. All help is much appreciated!

\$\endgroup\$
  • \$\begingroup\$ @TobySpeight Edited. I think it's more in line now - do you concur? \$\endgroup\$ – tonfred Oct 18 '18 at 18:25
  • \$\begingroup\$ Good edit - it's much better now! \$\endgroup\$ – Toby Speight Oct 18 '18 at 19:53
4
\$\begingroup\$

You've got a reasonable implementation for steps 1-3. I see no glaring issues with the code. Perhaps, we can over-engineer some parts of it.

General

Add docstrings.

class Card

It is mutable. I can be dealt the '6 of Spades' and by tweaking it card.rank = "Ace" all of a sudden have a better card for my hand than I was dealt. Trick decks aside, cards should be immutable, so we could change this into a tuple to keep in from changing. Since we want named fields, we need a namedtuple, and since we want to customize the str(card), we'll derive our own class from the namedtuple.

from collections import namedtuple

class Card(namedtuple("Card", ['rank', 'suit'])):
    __slots__ = ()   # Allows no additional fields, keeps memory footprint small

    def __str__(self):
        return f"{self.rank} of {self.suit}"

# Use like:
ace_of_spades = Card('Ace', 'Spades')

Note: I'm using the Python3.6 f-strings. If you're not using 3.6+, you'll have to adapt it to .format() ... or upgrade ;-)

Rank & Suit

We only want "Clubs", "Diamonds", "Hearts", and "Spades" as suits. And we only want "Ace", "2", "3" ... "10", "Jack", "Queen", and "King" as ranks.

Perhaps we might want an Enum for suit, and (maybe) another Enum for rank.

from enum import Enum

Suit = Enum('Suit', 'Clubs Diamonds Hearts Spades')
Rank = Enum('Rank', 'Ace Two Three Four Five Six Seven Eight Nine Ten Jack Queen King')

card = Card(Rank.Ace, Suit.Spades)
print(card)

Ew. This prints "Rank.Ace of Suit.Spades". We can fix this by changing the __str__(self) function of Card to include the .name of the rank and suit fields:

    def __str__(self):
        return f"{self.rank.name} of {self.suit.name}"

Much better.

But do we want to do this? It restricts the possible ranks and suits of our cards. If you wanted a Tarot deck, we might want the Cups, Wands, Pentacles and Swords for our suits. There are other specialty decks too, so maybe this is too restrictive. In which case, feel free to ignore this whole section. But if so, then add something like:

SUITS = ['Clubs', 'Hearts', 'Diamonds', 'Spades']

so you can iterate over a named constant, instead of needing to spell out the valid values each time you need them.

class Deck

What is a deck of cards? Is it always 52 standard playing cards? You can shuffle a deck and deal from a deck. But you can also shuffle two decks together and get 104 cards to deal from (perhaps you're playing Kanasta! No, wait, you'd need the Jokers too). What would you call two decks of cards mixed together? Is it still a deck? Maybe you want to pass some creation function into the Deck initializer, to control how it is created, instead of always building a deck of standard playing cards. Again, just a thought.

With the Enums created above, Deck.build() can be simplified:

def build(self):
    self.cards = [ Card(rank, suit) for rank in Rank for suit in Suit ]

Shuffling the cards: def shuffle(self, num=1). What is the num for? It isn't used.

Dealing the cards. Ok. It deal can deal 3 player a hand of 5 cards each. Good. Now, player #1 wants to draw a card ... and they get ... uhm ... one of their first cards again??? The deal() method needs to keep track of which cards you've already distributed, and which remain in the deck.


Is a hand of cards just a list of 5 cards? If so, why isn't a Deck just a list of 52 cards? Perhaps you need to deal the cards into a Hand class. The Hand class would be a good container for evaluating the hand, discarding cards and adding additional drawn cards to, etc.

\$\endgroup\$
  • \$\begingroup\$ Thanks, great advice and good sparring, many good questions as well! The aim is to keep everything very basic, so no more than 52 cards and no other suits than those of a standard deck (Spades etc.). Also no drawing new cards, only the 5 for each hand. But good point, need to keep track of which cards have been distributed. Shuffling: as I mentioned, a lot of the code is extracted from others and instructional videos. There was num in one, I thought it might be necessary so I kept it. Good to know it isn't! Hand class: I'll keep that in mind! \$\endgroup\$ – tonfred Oct 19 '18 at 14:51
  • \$\begingroup\$ I started out with "perhaps we can over-engineer it" because I didn't expect many of the suggestions would be followed. You stated you were a beginner, and I took you down the rabbit hole of immutability, namedtuples, and Enums. Hope you at least enjoyed the journey. \$\endgroup\$ – AJNeufeld Oct 19 '18 at 15:10
  • \$\begingroup\$ Each and every single word of advice and way of improvement is precious to me, they're what's helping me improve my knowledge. So thanks again, I did indeed enjoy the journey! \$\endgroup\$ – tonfred Oct 19 '18 at 15:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.