6
\$\begingroup\$

I've recently started learning how to code in Python, and have even more recently learned the basics of object-oriented programming. Feeling inspired, I started on a program that would generate random cards. I'm really excited to have completed a project like this for the first time!

I had a problem of duplicates, which was quickly solved by @user2357112 when they suggested I create a deck list.

After that, it was smooth sailing. Being a relatively new programmer though, I thought I'd get some suggestions for making the code more Pythonic, decreasing any repetition (which I kept minimal), using easier methods to do the same thing, etc.

I will also take any suggestions on code organization; being largely self-taught, my code may not be laid-out neatly as it could be.

# This program is practice for using classes, and doubles as a useful card deck program. Hopefully this can be of some use.
# Made by [brilliantlyInsane].

from random import randint as rand
from random import shuffle

suits = ("Spades","Hearts","Clubs","Diamonds")

class Card:
    def __init__(self, rank, suit):
        if rank not in range(1, 14):
            raise TypeError('Rank must be an integer between 1 and 13.')
        if suit not in suits:
            raise TypeError('Suit must be a string: "Spades", "Hearts", "Clubs", or "Diamonds".')
        # The quick check above makes sure the card being made actually exists in a standard deck of 52.
        # If so, the card is created succesfully.
        self.rank = rank
        self.suit = suit


    def cardName(self):
        """
        Returns a string containing the card's name in common terms.
        """
        if self.rank == 1:
            trueRank = "Ace"
        elif self.rank == 11:
            trueRank = "Jack"
        elif self.rank == 12:
            trueRank = "Queen"
        elif self.rank == 13:
            trueRank = "King"
        else:
            trueRank = str(self.rank)
        return "{rank} of {suit}".format(rank = trueRank, suit = self.suit)

    def flip(self):
        """
        Reveals the requested card.
        """
        print(self.cardName())

def newDeck():
    """
    Resets the deck to ascending order, containing all 52 cards.
    """
    global cardDeck
    cardDeck = [Card(rank, suit) for suit in suits for rank in range(1, 14)]
    cardDeck.reverse() # So that the bottom of the list is the top of the deck, i.e. the Ace of Spades is drawn first by 'cardDeck.pop()'.

newDeck()   # To generate the deck at the start. Note that it is not shuffled at first.

def shuffleDeck():
    """
    Self-explanatory. Shuffles the deck.
    """
    global cardDeck
    for i in range(0, 3):
        shuffle(cardDeck)   # Python's pseudorandom generator is slightly patterned unless shuffled multiple times.

def draw():
    """
    Draws a single card to a variable.
    Useful for replacing and discarding individual cards in a hand, such as replacing cards in poker.
    To do so: <hand>[<card to replace>] = cards.draw()
    Remember that the list for a hand starts from 0, not 1.
    """
    randCard = cardDeck.pop()
    return randCard

def drawFaceUp():
    randCard = cardDeck.pop()
    randCard.flip()
    return randCard

def drawHand(size):
    """
    Draws a <size>-card hand from the deck.
    """
    return [draw() for i in range(0, size)]

def showHand(hand):
    size = len(hand)
    for i in range(0, size):
        hand[i].flip()

def newCard():
    """
    Generates a random card outside of the established deck, and prints its value.
    While occasionally useful, using newCard() for hands is discouraged. Duplicates of preexisting cards will result.
    """
    suit = suits[rand(0, 3)]
    rank = rand(1,13)
    randCard = Card(rank,suit)
    print("The {card} has been generated.".format(card = str(randCard.cardName())))
    return randCard

def cardHelp():
    """
    Gives a set of instructions explaining the use of the 'cards.py' module.
    """
    print('\n' + '=' * 72)
    print('=' * 13 + " [brilliantlyInsane]'s Python Cards: Instructions " + '=' * 14)
    print('=' * 72 + '\n')

    print('—' * 16 + " The Cards " + '—' * 45)
    print('—' * 72)
    print('The "Card" object has two attributes:')
    print('rank - An integer between 1 and 13. (Ace = 1, Jack = 11, Queen = 12, King = 13.)')
    print('suit - A string value of either "Spades", "Hearts", "Clubs", or "Diamonds".')
    print('A specific card object can be made: "Card(<rank>,<suit>)".\n')

    print('—' * 16 + " Drawing Cards " + '—' * 41)
    print('—' * 72)
    print('"Draw" cards to a variable with "<var> = cards.draw()".')
    print('Use "cards.drawFaceUp() to draw a card and print its value.')
    print('"Flip" a card (print its value) with "<var>.flip()".')
    print('Generate an entirely new random card using "cards.newCard()".')
    print('(Note that "newCard()" duplicates a card in the deck.)\n')

    print('—' * 16 + " Hands " + '—' * 49)
    print('—' * 72)
    print('To draw an entire hand with <size> many cards, use "cards.drawHand(<size>)".')
    print('To show a hand, use "cards.showHand(<hand>)."\n')

    print('—' * 16 + " Replacing Cards " + '—' * 39)
    print('—' * 72)
    print('You can replace individual cards in a hand using <hand>[card #] = cards.draw().')
    print('However, lists in Python start FROM 0, not 1!')
    print('"hand[1] = cards.draw()" will replace the SECOND card in your hand, not the first!\n')

    print('—' * 16 + " The Deck " + '—' * 46)
    print('—' * 72)
    print('The deck is stored to a list under the variable "cards.cardDeck".')
    print('Shuffle using "shuffleDeck()". The deck is unshuffled by default.')
    print('Reset the deck completely using cards.newDeck().')
    print('\n' + '=' * 72 + '\n')

print('Type "cards.cardHelp()" to learn how to use this module.')
\$\endgroup\$
4
\$\begingroup\$

Some comments:

Check out PEP8

PEP8 is like the style guide of Python. You don't need to adhere to it, but most people are used to reading code that does.

Avoid global variables

Like @RUser4512 mentioned, go OOP, thus avoiding global variables.

Your code will look like this:

class Card:
    def __init__(self, rank, suit): pass
    def name(self): pass
    def flip(self): pass

class Deck:
    def __init__(self): pass  # newDeck()
    def shuffle(self): pass
    def draw(self): pass
    def draw_hand(self, size): pass
    def draw_faceup(self): pass

def show_help(): pass

Using idioms like __str__()

You might want to change name() to __str__(). Then you can use str(card) to get the name. If I were you, unless you plan to implement additional behaviour for flip(), I would just avoid it and use print(card) to print the card info.

Don't print text on module load

Avoid printing anything when someone imports your module. The users of your library might not appreciate this. Instead, use a docstring at the start of your module.

If you do want to write a function to print the help, use functions

print('—' * 16 + " Hands " + '—' * 49)
print('—' * 72)

is more readable -- and easier for you to write ;) -- when replaced by

def print_line(text=None):
    if text:
        output = '-' * 16 + " {} ".format(text)
        output = output.ljust(72, "-")
    else:
        output = "-" * 72
    print(output)

print_line("Hands")
print_line()
\$\endgroup\$
3
\$\begingroup\$

I am not a Python expert but I have some comments.

The case style. Most Python users prefer using underscores instead of upper case letters. See : What is the naming convention in Python for variable and function names?

Global variables:

def newDeck():
    """
    Resets the deck to ascending order, containing all 52 cards.
    """
    global cardDeck
    cardDeck = [Card(rank, suit) for suit in suits for rank in range(1, 14)]
    cardDeck.reverse() # So that the bottom of the list is the top of the deck, i.e. the Ace of Spades is drawn first by 'cardDeck.pop()'.

Do you need a global variable ? Can't you just put the deck you draw the card from in the argument of the drawCardFromDeck function ?

You could change the other method, like

def drawCardFromDeck(cardDeck):
    """
    Draws a single card to a variable.
    Useful for replacing and discarding individual cards in a hand, such as replacing cards in poker.
    To do so: <hand>[<card to replace>] = cards.draw()
    Remember that the list for a hand starts from 0, not 1.
    """
    return cardDeck.pop()

To emphasize the fact that cardDeck is modified when this method is called.

Go OOP

But there is another problem with the global deck. What happens if you want to have another deck (for some reason). All these would be even better if Deck was itself a class with methods:

  1. __init__ your current newDeck()
  2. draw(), drawHand(size), drawFaceUp(), shuffleDeck() would be method from this deck class and it would be more legitimate for them to change the state of the deck.
\$\endgroup\$
  • \$\begingroup\$ drawCardFormDeck() [sic] should just return randCard. cardDeck is mutated, so there is no need to return it. Returning a tuple just makes the function hard to use. \$\endgroup\$ – 200_success Mar 4 '16 at 10:42
  • \$\begingroup\$ @200_success, you are right, fixed ! \$\endgroup\$ – RUser4512 Mar 4 '16 at 10:44
  • \$\begingroup\$ You might as well drop randCard altogether and just return cardDeck.pop(). \$\endgroup\$ – 200_success Mar 4 '16 at 10:46
  • \$\begingroup\$ @200_success edited as well \$\endgroup\$ – RUser4512 Mar 4 '16 at 10:47
1
\$\begingroup\$
def cardName(self):
        """
        Returns a string containing the card's name in common terms.
        """
        if self.rank == 1:
            trueRank = "Ace"
        elif self.rank == 11:
            trueRank = "Jack"
        elif self.rank == 12:
            trueRank = "Queen"
        elif self.rank == 13:
            trueRank = "King"
        else:
            trueRank = str(self.rank)
        return "{rank} of {suit}".format(rank = trueRank, suit = self.suit)

This can be shortened, simplified (and made more pythonic):

specialCards = {11: "Jack", 12: "Queen", 13:"King", 1: "Ace"}

def generateCardName(self):
  name = specialCards[self.rank] if rank in specialCards else rank
  return '%s of %s'(name, self.suit)

But like someone said above, go OOP.

So I would have

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


  def cardName(self):
    specialCards = {11: "Jack", 12: "Queen", 13:"King", 1: "Ace"}
    rank_name = specialCards[self.rank] if self.rank in specialCards else self.rank
    return '%s of %s'%(rank_name, self.suit)

Instantiate it like this:

my_card = Card(1,"Spades")
print my_card.cardName()

For the whole card deck:

suits = ["Spades", "Hearts", "Diamonds", "Clubs"]
cards = [Card(rank, suit) for rank in range(1,14) for suit in suits]
\$\endgroup\$

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.