8
\$\begingroup\$

I am looking for some feedback on my blackjack Python code. Any feedback is much appreciated. Docstring, code etc.

Other than that, I need to test it, and I am not sure how to make this code more testable. Any tips on that would also be appreciated.

from random import shuffle

class Card():
    """Class handles cards in a deck

     Attributes:
        suit       --  The possible suits in a deck of cards
        card_value --  The possible values in a deck of cards
    """

    suits = [('Heart',1), ('Diamond',2), ('Spade',3), ('Club',4)]
    values = [('Ace',11),('Two',2),('Three',3),('Four',4),('Five',5),
     ('Six',6),('Seven',7), ('Eight',8), ('Nine',9), ('Ten',10),
     ('Jack',10), ('Queen',10),('King',10)]

    def __init__(self, card_value = 0, suit = 0):
        """Inits Card class with card_value and suit """
        self.card_value = Card.values[card_value]
        self.suit = Card.suits[suit]

class Entity():
    """Class handles entities and game logic

     Attributes:
        bet_account: int     --  holds the players account amount
        entity_name: string  --  holds the name of the player
        cards: list          --  holds the cards
    """


    def __init__(self, bet_account = 0, entity_name = 'name'):
        """Inits Enitity class with bet_account, entity.name and cards """
        self.bet_account = bet_account
        self.entity_name = entity_name
        self.cards = []


    def deposit(self, amount):
        """deposit momey into players account

        Parameters:
        amount: int -- amount to deposit
        """
        self.bet_account += amount


    def calc_card_value(self):
        """calculates the total value of a players cards, and handles aces

        Returns:
        the total value of player or house cards. 
        """
        total_value = 0
        for card in self.cards:
            total_value += card.card_value[1]

        #checks for aces, and adjust accordingly
        if total_value > 21:
            for card in self.cards:
                if card.card_value[0] == "Ace":
                    total_value -= 10
                if total_value < 21:
                    break

        return total_value

    def print_current_cards(self):
        """prints the current cards on the table to the terminal"""
        print('---------------------------------')
        print(f'{self.entity_name.capitalize()}''s cards:')
        for card in self.cards:
            print(f'Card: {card.card_value[0]}, of {card.suit[0]}')
        print(f'Total card value: {self.calc_card_value()}')
        print('---------------------------------')


def generate_deck():
    """Generate a deck of cards.

    Returns:
    a new deck containing 52 cards
    """
    deck = []
    order = list(range(1,53))
    shuffle(order)
    for i in order:
        card = Card(i % 13, i % 4)
        deck.append(card)

    return deck

def deal_card(player, deck):
    """gets a card out of the deck, to hand over to player 

    Parameters:
    player: obj -- object of player
    deck: list -- list of the deck
    """
    player.cards.append(deck.pop())

def check_winner(player, house, bet):
    """Check who won the game by going through the scores and dertimining who won """

    if house.calc_card_value() == 21:
        print("House got blackjack!")
    if player.calc_card_value() == 21:
        print(player.entity_name + " got blackjack!")

    if house.calc_card_value() > 21:
        print(player.entity_name + " won")
        player.deposit(bet)

    elif player.calc_card_value() > house.calc_card_value():
        print(player.entity_name + " won")
        player.deposit(bet)
    elif player.calc_card_value() == house.calc_card_value():
        print("Tie!")
    else:
        print('House won')
        player.deposit(-bet)  


def play_game(player, house, deck, bet):
    """
    Game functionality; deals cards,
    handles hit and pass, 
    checks if player busts 

    Parameters:
    player: obj -- player object
    house: obj -- house object
    deck: list -- list of deck
    bet: int -- placed bet
    """

    #deals 2 cards to the player, and one for the dealer
    deal_card(house, deck)
    deal_card(player, deck)
    deal_card(player, deck)
    #prints the current card on the table
    player.print_current_cards()
    house.print_current_cards()
    bust = False
    #get user input. 
    #if user busts, bust is set to True, and the player looses their bet
    #if the user decides to hit, they are dealt another card. 
    while True:
        action = input('(h (hit) or s (stand)?')
        if action == 'h':
            deal_card(player, deck)
            player.print_current_cards()

        elif action == 's':
            player.print_current_cards()
            break

        if player.calc_card_value() > 21:
            player.print_current_cards()
            print(player.entity_name + ' busts')
            bust = True
            break

    if bust:
        player.deposit(-bet)
    #computers turn if the user decides to stand
    else:
        while house.calc_card_value() < 17:
            deal_card(house, deck)

    house.print_current_cards()

    if not bust:
        check_winner(player, house, bet)

    print(f'{player.entity_name} you now have {player.bet_account} in your account')


def main():
    """Initial setup. Gets player name and how much they wants to deposit, starts game """
    print()
    name = input('What is your name?').capitalize()
    if name == "":
        print("You need to type a name")
        main() 
    try:       
        money = int(input('How much do you want to deposit?'))
        if money <= 0:
            print("deposit must be bigger than 0, starting over..")
            main()
    except ValueError:
        print("Not a valid deposit. Starting over..")
        main()
    #creates objects of player and house
    player = Entity(bet_account = money, entity_name = name)
    house = Entity(bet_account = 10000000, entity_name = "House")
    stop = False
    while not stop:
        deck = generate_deck()
        player.cards = []
        house.cards = []
        try:
            bet = int(input('How much do you want to bet?'))
            if bet <= player.bet_account and bet > 0:
            # starts the game
                play_game(player,house,deck,bet) 
            else:
                print("Bet cannot be bigger than what you have, and cannot be 0")
        except ValueError:
            print("Not a valid bet")       

        want_to_stop = input('To stop write ¨s¨, to try again press enter')
        if want_to_stop == "s":
            stop = True



if __name__ == '__main__':
    main()
\$\endgroup\$
7
\$\begingroup\$

Card:

  1. Consider renaming card_value to just value. Since it belongs to an instance of the Card class, there is no point in restating the "card" part.

  2. What purpose does the suits list of name-value pairs serve? It seems it's only used once in your script. In fact, it doesn't make sense to associate a suit with a number, especially since you just rely on indices later in generate_deck and never actually use the number portion of the pairs.

  3. Consider changing the values list to a dictionary that maps a numerical value to its corresponding string name. For example:

values = { 1: 'Ace', 2: 'Two', 3: 'Three', 4: 'Four', ... }

This would eliminate your reliance on passing in indices to initialize a card with a particular value, since that logic is a bit convoluted as it requires decrementing a card's value by one (or, in the case of Ace = 11, simply doesn't make sense).

Refactor your code for this accordingly.

Entity:

  1. Why not rename this to Player? In game dev, you usually only give something the generic name of Entity if that class is going to be an abstract class. Otherwise, you could argue that anything is an entity, including a card. Technically, the house is also a Player.

  2. You are currently using deposit for both positive and negative values. That works and is fine from a technical standpoint, but it's not from a logical one. Consider using two separate methods, deposit and withdraw, that serve opposite purposes.

  3. calc_card_value isn't informative—what you're really doing is computing a hand_value. Moreover, your logic/efficiency can be simplified quite a bit, especially if you use my dictionary suggestion above for Card:

total_value = 0

for card in self.cards:

    if total_value > 21 and card.value == 'Ace':
        total_value -= 10
    else:
        total_value += card.value

return total_value

This eliminates unnecessarily looping over the cards twice and is a bit more compact. A downside/tradeoff is that you're checking the conditional on each iteration.

EDIT: This is a classic example of trying to optimize something and breaking the code in the process. I am wrong—you do need two loops here. My code doesn't do what you'd expect.

Instead, to keep the code more compact while still using two loops, I'd use a list comprehension:

total_value = sum([card.value for card in self.cards])

if total_value > 21:
     for card in self.cards:
          if card.card_value == "Ace":
               total_value -= 10
          if total_value <= 21:
               break

Global methods

  1. generate_deck is good.

  2. deal_card makes more sense as part of a BlackjackGame class that manages the game (more on this later/throughout the rest of the review).

  3. Let's take a look at check_winner:

    a. The logic would be much cleaner if this were part of a class like BlackjackGame that has members for the house and the player.

    b. You should define a class constant in BlackjackGame for the "magic number" of 21: BLACKJACK = 21. That way, everyone can reference it as BlackjackGame.BLACKJACK (however, this isn't that big of a deal; 21 is well known as Blackjack in... Blackjack!).

  4. play_game would also be much simpler if it were part of BlackjackGame. You could then just rename it to play, deal the cards to the members self.house and self.player, and so on.

The code is also a bit long for this function, mainly because you're doing two things: taking input for the player, and then taking "input" for house/the computer. Consider splitting those two code segments into their own methods and calling them here one after the other. The method for the player's scenario could return a boolean indicating whether the player bust or not. So something like:

#deals 2 cards to the player, and one for the dealer
deal_card(house, deck)
deal_card(player, deck)
deal_card(player, deck)
#prints the current card on the table
player.print_current_cards()
house.print_current_cards()
#runs player and house scenarios
player_bust = self.determine_player_outcome()

if player_bust:
    player.withdraw(bet)
else:
    cycle_dealing_to_house()
    self.check_winner(bet)

Notice how your conditional for if not bust was also redundant—that's already covered by the else after if bust.

Main

  1. Your logic for getting the player's name is really convoluted. You're calling main from within main... Don't you end up with two instances of the game running? Instead, do this:

    a. Split this into a global function called get_player_name():

    while True:
        name = input('What is your name?').capitalize()
        if name is not "":
            return name
        else:
            print("You need to type a name!")
    
    

    Then, main doesn't need to care about these details anymore; it just calls get_player_name and is guaranteed to either cycle the print statement or get a valid name and continue.

  2. Same goes for depositing money—create a function named ask_player_to_deposit_money or something shorter, and run a similar loop inside its body. Then call it inside main and store the return value in a variable.

  3. Remember to create a BlackjackGame class and consolidate the player/house construction in there.

  4. The rest of the code is fine.

Testing

I recommend looking into the unittest framework for Python. Unit testing allows you to isolate functionally independent parts of a program and to test them separately from the rest to ensure that they function as intended.

You can of course also test the game manually by playing it, but it would be tedious to run through every scenario by hand.

\$\endgroup\$
  • \$\begingroup\$ in the case of Ace = 11, simply doesn't make sense: I believe that this is so that comparators (>, <, ...) give the ace the highest in-game value. \$\endgroup\$ – PiCTo May 27 at 10:12
  • 1
    \$\begingroup\$ Oh, no, to clarify, what I meant is that using indices to get the card values does not consistently make sense. For values 2-10, it's a matter of values[n-1] = card with value of n. But the Ace is an exception to this rule, so I wanted to emphasize that a map would make this even clearer. \$\endgroup\$ – AleksandrH May 27 at 13:54

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.