2
\$\begingroup\$

I recognize that this is a common question area with lots of answers: Here and here for example.

Specifically I'm trying to better understand how to use Enums in Python. Suits in a deck of cards seems appropriate, but implementing them as a class seems extra, and I'm curious if there's a more pythonic way to do it?

class Suits(Enum):
    Club = 1
    Heart = 2
    Diamond = 3
    Spade = 4

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

class Deck:
    def __init__(self):
        self.cards = self.initDeck()

    def initDeck():
        cards = {}
        for suit in Suits:
            for i in range(14):
                cards.add(Card(i, suit))
        return cards

The whole point of Enums is to prevent you from adding a bad value though, right? (Like, making a new card as Card(0, 'joker')) so maybe this isn't the right application for it? Currently there would be no error if I passed Card(4, 'dimond') and that's exactly what enums are supposed to prevent, right?

Hoping for a bit more insight/education. Thanks!

\$\endgroup\$

closed as unclear what you're asking by πάντα ῥεῖ, Graipher, Sᴀᴍ Onᴇᴌᴀ, Ludisposed, Mast Jan 14 '18 at 20:35

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ That is one of the reasons for enums yes so I do think it is appropriate. As for a joker card - I think it is better to have a base class (e.g. Card, abstract if Python supports it), then two sub classes - JokerCard and SuitCard. That way you don't have to specify rank or suit for JokerCards. Also - strongly suggest avoiding magic numbers like "14". \$\endgroup\$ – JanErikGunnar Jan 13 '18 at 20:17
  • \$\begingroup\$ For the record, I didn't vote to close because it was unclear what you asked. It's perfectly clear. However, it's example code instead of a review of actual code in actual context. It smells like one of those best practices in general questions which shouldn't be asked here. \$\endgroup\$ – Mast Jan 14 '18 at 20:37
  • \$\begingroup\$ @Mast that's fair, this IS a best practices in general/"how do I use Enums" question. I asked it over in SO and was directed here. So worries on the vote to close \$\endgroup\$ – singmotor Jan 14 '18 at 22:55
  • \$\begingroup\$ @Acoustic77 Unfortunately there's plenty of people on SO redirecting unsuspecting users here without having a clue what Code Review is about. \$\endgroup\$ – Mast Jan 14 '18 at 23:59
3
\$\begingroup\$

Your implementation uses Enum for the suits, but not for the values. Thus it is harder to validate the card values, and would as you noted, allow a joker card to be created, even though you didn't want that ability.

Card Value Enum:

A card value enum provides the ability to give the cards a value and a descriptive name, while limiting possible values to valid ranges:

class CardValue(Enum):
    Ace = 1
    Deuce = 2
    Three = 3
    Four = 4
    Five = 5
    Six = 6
    Seven = 7
    Eight = 8
    Nine = 9
    Ten = 10
    Jack = 11
    Queen = 12
    King = 13

Immutable Types

For something like a card, an immutable type can provide some advantages. One big one, is that the same value and suit will always key the same in a set or dict. Inheriting from tuple will achieve imuutability. Note that the class is setup in __new__ not __init__ since it is immutable.

class Card(tuple):

    def __new__(cls, value, suit):
        assert isinstance(value, CardValue)
        assert isinstance(suit, CardSuit)
        return tuple.__new__(cls, (value, suit))

property decorator

The property decorator makes it easy to access the value and suit.

    @property
    def value(self):
        return self[0]

    @property
    def suit(self):
        return self[1]

__str__ method

Since we are using Enum's for all of the values, a fully descriptive __str__ method is quite straight forward:

    def __str__(self):
        return "{} of {}s".format(self.value.name, self.suit.name)

So something like:

print(Card(CardValue.Ace, CardSuit.Club))

gives:

Ace of Clubs

Make sure we stay immutable

Suggest adding some boilerplate to help avoid abusing the class instances.

    def __setattr__(self, *ignored):
        raise NotImplementedError

    def __delattr__(self, *ignored):
        raise NotImplementedError

Deck class

So the Deck class can be made much simpler with the new Value class and a set comprehension like:

class Deck:
    def __init__(self):
        self.cards = {
            Card(value, suit) for value in CardValue for suit in CardSuit
        }

Whole Listing:

from enum import Enum

class CardValue(Enum):
    Ace = 1
    Deuce = 2
    Three = 3
    Four = 4
    Five = 5
    Six = 6
    Seven = 7
    Eight = 8
    Nine = 9
    Ten = 10
    Jack = 11
    Queen = 12
    King = 13


class CardSuit(Enum):
    Club = 1
    Heart = 2
    Diamond = 3
    Spade = 4


class Card(tuple):

    def __new__(cls, value, suit):
        assert isinstance(value, CardValue)
        assert isinstance(suit, CardSuit)
        return tuple.__new__(cls, (value, suit))

    @property
    def value(self):
        return self[0]

    @property
    def suit(self):
        return self[1]

    def __str__(self):
        return "{} of {}s".format(self.value.name, self.suit.name)

    def __setattr__(self, *ignored):
        raise NotImplementedError

    def __delattr__(self, *ignored):
        raise NotImplementedError

class Deck:
    def __init__(self):
        self.cards = {
            Card(value, suit) for value in CardValue for suit in CardSuit
        }
\$\endgroup\$

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