3
\$\begingroup\$

There are multiple aspects in the code I do not really like. [card for card in ...] looks really lame, also [x.pop() for i in range(y)]. I'm looking forward for any recommendation.

from random import shuffle 

class Deck(object):
    suits = range(4)
    ranks = range(13)

    @classmethod
    def generator(cls, suits, ranks):
        for suit in suits:
            for rank in ranks:
                yield({'suit': suit, 'rank': rank})

    def __init__(self):
        self.cards = [card for card in Deck.generator(Deck.suits, Deck.ranks)]
        shuffle(self.cards)

    def deal(self, amount):
        return [self.cards.pop() for i in range(amount)]

print Deck().deal(5)
\$\endgroup\$
2
\$\begingroup\$

[card for card in Deck.generator(…)] could be written as list(Deck.generator(…)).

Better yet, use itertools.product():

self.cards = [
    {'suit': suit, 'rank': rank}
    for suit, rank in itertools.product(xrange(4), xrange(13))
]

In Python 2, you should be using xrange() rather than range().

It's probably worth defining a Card class. At some point, you'll want to have suits that are named rather than numbered, and ranks A, J, Q, K rather than 0..12.

To deal multiple cards, you can slice the list instead of popping one card at a time:

def deal(self, n):
    hand = self.cards[-1 : -n-1 : -1]
    self.cards = self.cards[: -n]
    return hand
\$\endgroup\$
  • \$\begingroup\$ 1, itertools.product() and the proposed list comprehension looks great, thank you for that. 2, I will look into that xrange() topic - I already saw it, but did not pay attention yet. 3, I do have a Card class with overridden __lt__, __gt__, __eq__ functions, but I did not want to clutter the question with it. 4, I decided not to use list slicing for the sake of simplicity, I will reconsider. Thank you :) \$\endgroup\$ – Lorinc Nyitrai Aug 7 '15 at 11:03
  • 1
    \$\begingroup\$ about the xrange() vs. range() subject: stackoverflow.com/a/97530/1486768 \$\endgroup\$ – Lorinc Nyitrai Aug 7 '15 at 12:13
  • \$\begingroup\$ Unless your lists are going in the millions of items, which they clearly are not here, I'd stick with range, mostly for portability. I similarly don't use any of the iterxxx methods of Python 2 dicts for the exact same reason. \$\endgroup\$ – Jaime Aug 7 '15 at 13:16
  • 1
    \$\begingroup\$ I can't resist noting that although the slicing is probably more efficient, it sort of violates the poker rule that cards must be dealt off the top of the deck, one at a time. The .pop() route is arguably the best simulation of this poker rule. You don't want any virtual card sharks accusing you of cheating... :-) \$\endgroup\$ – Curt F. Aug 8 '15 at 0:41
  • \$\begingroup\$ @CurtF. The way I wrote it, the slice is reversed, so the result should be the same. \$\endgroup\$ – 200_success Aug 8 '15 at 1:19

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.