Gfrktyl

I wrote classes to model a playing card, a deck, and a player to be used in multiple playing card games. I originally posted a Card class here, and now extended the project by adding a deck and player class and am really looking for some feedback. This is my first real object oriented design project and I feel unsure about the current way the deck and player classes are implemented.

class Card(object):

    """Models a playing card, each Card object will have a suit, rank, and weight associated with each.



    possible_suits -- List of possible suits a card object can have

    possible_ranks -- List of possible ranks a card object can have

    Suit and rank weights are initialized by position in list.

    If card parameters are outside of expected values, card becomes joker with zero weight

    """



    possible_suits = ["Clubs", "Hearts", "Diamonds", "Spades"]

    possible_ranks = [2, 3, 4, 5, 6, 7, 8, 9, 10, "J", "Q", "K", "A"]



    def __init__(self, suit, rank):

        if suit in Card.possible_suits and rank in Card.possible_ranks:

            self.suit = suit

            self.rank = rank

            self.suit_weight = Card.possible_suits.index(suit)

            self.rank_weight = Card.possible_ranks.index(rank)

        else:

            self.suit = "Joker"

            self.rank = "J"

            self.suit_weight = 0

            self.rank_weight = 0



    def __str__(self):

        """Returns abbreviated name of card



        Example: str(Card('Spades', 'A') outputs 'AS'

        """

        return str(self.rank) + self.suit[0]



    def __eq__(self, other):

        """Return True if cards are equal by suit and rank weight"""

        return self.suit_weight == other.suit_weight and self.rank_weight == other.rank_weight



    def __gt__(self, other):

        """Returns true if first card is greater than second card by weight"""

        if self.suit_weight > other.suit_weight:

            return True

        if self.suit_weight == other.suit_weight:

            if self.rank_weight > other.rank_weight:

                return True

        return False



    def modify_weight(self, new_suit_weight = None, new_rank_weight = None):

        """Modifies weight of card object"""

        if new_suit_weight:

            self.suit_weight = new_suit_weight

        if new_rank_weight:

            self.rank_weight = new_rank_weight



    def get_suit(self):

        return self.suit



    def get_rank(self):

        return self.rank



    def get_suit_weight(self):

        return self.suit_weight



    def get_rank_weight(self):

        return self.rank_weight



from itertools import product

from random import shuffle



class Deck(object):

    """Models a deck of playing cards, each instance will be a list of 52 Card objects"""



    def __init__(self):

        self.cards = [Card(card[0], card[1]) for card in product(Card.possible_suits, Card.possible_ranks)]



    def __str__(self):

        return str([str(card) for card in self.cards])



    def length(self):

        return len(self.cards)



    def draw(self, end = None):

        """Return a card object or a set of card objects from the zeroth index of the deck, while removing it"""

        card = self.cards[0: end]

        del self.cards[0: end]

        return card



    def shuffle_deck(self):

        """Shuffle the deck of cards in place"""

        shuffle(self.cards)



    def get_suit(self, suit):

        """Return all cards of suit in the deck"""

        cards_of_suit = 

        for card in self.cards:

            if card.get_suit() == suit:

                cards_of_suit.append(card)

            if len(cards_of_suit) == 13:

                break

        return cards_of_suit



    def find(self, card_to_find):

        """Return position of card in deck, if card not in deck return None"""

        print(type(card_to_find), "; ", card_to_find)

        if card_to_find in self.cards:

            return self.cards.index(card_to_find)

        return None



    def remove(self, card_to_remove):

        """Remove a card from the deck"""

        position = self.find(card_to_remove)

        if position:

            del self.cards[position]



class Player(object):

    """Models a Player in a playing card game, each instance will have an id,

    can be initialized with a hand of cards

    and can be initialized with a score

    """



    def __init__(self, id, hand = None, score = None):

        self.id = id

        self.hand = hand

        self.score = score



    def __str__(self):

        return str(self.id)



    def set_hand(self, cards):

        """Append cards to hand"""

        self.hand = cards



    def find_card(self, card):

        """Finds a card in hand"""

        if card in self.hand:

            return self.hand.index(card)

        return None



    def play(self, card_to_be_played):

        """Return card_to_be_played from hand while removing it, return None if card not in hand"""

        position = self.find_card(card_to_be_played)

        if position:

            del self.hand[position]

            return card_to_be_played

        return None



    def update_score(self, update):

        """Updates the current score of the instance by adding the update"""

        self.score += update



    def get_hand(self):

        return [str(i) for i in self.hand]



    def get_score(self):

        return self.score 

You may want to make the suits and ranks Enums, so that you can make cards in a more readable and less error prone way when needed:

Card(Suit.Spade, Rank.King)  # readable, clear

Card("Spade", "K")  # prone to typos 

Card(Card.possible_suits[0], Card.possible_ranks[11])  # meaning not very clear

It could also be beneficial to add a dict to your Deck implementation to more efficiently query the randomized deck:

# quickly retrieve cards of a suit

suitToCards = { 

    Suits.Spade: filter(card.suit == Suits.Spade, self.cards)

    # repeat for all suits...

} 

This way the Deck.cards are randomized, but Deck.bySuit can be used internally by Deck.get_suit() and Deck.find and Deck.remove so you don't need to iterate over the entire deck when you know you only ever need to work with a portion of the deck for these operations.

One more thing to consider is encapsulation of the cards in the deck. You've handled Deck.draw() well, but it looks like cards in the deck can be altered or deleted if the caller manipulates the cards returned by Deck.get_suit(). You may want to make a deep copy of the list you return in Deck.get_suit() to prevent this.

If this is to model standard deck based games where a deck always has 1 of each card (and optional jokers), and a card will always be in the deck or not, you may want to re-evaluate your design where cards are deleted from the deck. Will you ever want to support shuffling a card or cards back into the deck once drawn? How would you prevent duplicate cards from ending up in the deck? It could be easier to add a Boolean to each card indicating if its in the deck or not. This way the deck always knows about the cards it was initialized with, and these classes will be easier to debug.