Simpler version of BlackJack game - first OOP

Question

I am learning Python for a few weeks. Last time I started object-oriented programming and I feel like it is really hard for me to write the relation between methods and attributes for different objects.

I tried to write simpler version BlackJack card game only with Hit or Stand option.

Could you have a look at my code and possibly give me some advice what I should change or improve?

import random

SUITS = ['Heart','Diamond','Club','Pike']
FIGURES = ['A','2','3','4','5','6','7','8','9','10','J','Q','K']
MONEY = 200
MIN_BET = 2

class Card:
    '''
    Single card class
    Use it in loop to make full french deck of card
    print return 'Figure of suit'
    '''

    def __init__(self,suit,figure):
        self.suit = suit
        self.figure = figure

        if self.figure == "A":
            self.value = 11
        elif self.figure in ['J','Q','K']:
            self.value = 10
        else:
            self.value = int(self.figure)

    def __str__(self):
        return f'{self.figure} of {self.suit}'

    def __repr__(self):
        return f'{self.figure} of {self.suit}'


class Deck:

    def __init__(self):
        self.deck = [Card(suit, figure) for suit in SUITS for figure in FIGURES]

    def shuffle(self):
        random.shuffle(self.deck)

    def draw(self):
        return self.deck.pop(0)

    def restart(self):
        self.deck = [Card(suit, figure) for suit in SUITS for figure in FIGURES]

class Hand:
    def __init__(self):
        self.cards = []
        self.value = 0
        self.aces = 0    

    def add_card(self, card):
        self.cards.append(card)
        self.check_aces()

    def check_value(self):
        '''
        Change self.value
        Doesnt return any value
        '''
        ace = self.aces
        self.value = 0

        for i in range(len(self.cards)):
            self.value += self.cards[i].value

            while self.value > 21 and ace > 0:
                self.value -= 10
                ace -= 1

        if self.value > 21:
            self.isBusted = True

        return self.value

    def check_aces(self):
        '''
        Check if 'A' in Hand 
        '''
        self.aces = 0

        for i in range(len(self.cards)):
            if self.cards[i].figure == 'A':
                self.aces += 1

    # I know i should make new class dealer inherit from Hand with this function
    def dealer_ai(self,d):
        self.check_value()
        while self.value < 17:
            self.add_card(d.draw())
            self.check_value()
        print(f'Dealer cards: {self.cards} ')

    def restart(self):
        self.cards = []
        self.value = 0
        self.aces = 0  

class Bank:

    def __init__(self, money, min_bet):
        self.money = money
        self.min_bet = min_bet
        self.play = True
        self.pool = 0

        self.check_money()
        print(self)


    def __str__(self):
        return f'You have {self.money}$'

    def check_money(self):
        if self.money < self.min_bet:
            self.play = False
        else:
            self.play = True

    def bet(self):
        print(f'You have {self.money}$.')

        while True:
            try:
                bet = int(input('Tell me your bet (should be lower or equal then your money)'))
            except:
                continue
            else:
                break

        if bet>self.money:
            self.bet()

        else:
            self.pool = 2*bet
            self.money -= bet
            print(f'Your new balance: {self.money}$')

    def deposite(self,depo):
        self.money += depo

    def player_win(self):
            self.deposite(self.pool)
            print(f'You won: {self.pool}$ \nYour new balance is {self.money}.')
            self.pool = 0

    def player_lose(self):
            print(f'Sorry! You lose {self.pool/2}.\nYour new balance is {self.money}')
            self.pool = 0


def play_choice():
    choice = ''
    while True:
        choice = input('Do you want to play? Y or N: ')
        if choice.lower() == "y":
            return True
        elif choice.lower() == 'n':
            return False
        else:
            play_choice()

def main():

    print('Welcome in Blackjack game! There is only two options - hit or stand')
    print('If you dont know rules - google for Blackjack')

    ##Creating Objects
    deck = Deck()
    player_hand = Hand()
    dealer_hand = Hand()
    bank = Bank(MONEY,MIN_BET)

    winner = True
    is_playing = play_choice()
    ##1 round -> Draw, hit/stand -> dealer -> check if win   
    while is_playing:


        while True:
            deck.restart()
            player_hand.restart()
            dealer_hand.restart()
            deck.shuffle()
            bank.bet()

            #Lets draw 4 cards - 2 For you, 2 for Dealer
            player_hand.add_card(deck.draw())
            player_hand.add_card(deck.draw())
            dealer_hand.add_card(deck.draw())
            dealer_hand.add_card(deck.draw())
            player_hand.check_value()
            print(f'\n\nYour cards: {player_hand.cards} with {player_hand.value} value')
            print(f'Dealer first card: {dealer_hand.cards[0]} ')
            choice = input('\n\nWhats your choice? - "h" for hit / "s" for stand')
            while choice.lower() == 'h':
                player_hand.add_card(deck.draw())
                player_hand.check_value()
                print(f'\n\nYour cards: {player_hand.cards} with {player_hand.value} value')
                if player_hand.value >= 21:
                    break
                choice = input('\n\nWhats your choice? - "h" for hit / "s" for stand')

            ###
            ## Check for player value
            if player_hand.value == 21:
                winner = True
                break
            elif player_hand.value > 21:
                winner = False
                break

            dealer_hand.dealer_ai(deck)


            if dealer_hand.value > 21:
                winner = True
                break
            elif dealer_hand.value == 21:
                winner = False
                break

            elif player_hand.value > dealer_hand.value:
                winner = True
                break
            else:
                winner = False
                break

        if winner:
            bank.player_win()
        else:
            bank.player_lose()


        is_playing = play_choice()

if __name__ == "__main__":
    main()

Answer 1

Confusing documentation and lack of consistency

def check_value(self):
    '''
    Change self.value
    Doesnt return any value
    '''
    ace = self.aces
    self.value = 0

    for i in range(len(self.cards)):
        self.value += self.cards[i].value

        while self.value > 21 and ace > 0:
            self.value -= 10
            ace -= 1

    if self.value > 21:
        self.isBusted = True

    return self.value

def check_aces(self):
    '''
    Check if 'A' in Hand 
    '''
    self.aces = 0

    for i in range(len(self.cards)):
        if self.cards[i].figure == 'A':
            self.aces += 1

In check value you say "Doesn't return any value" but it clerly has a return statement inside. It is important that code and comments never contradict each other. In your code you do not use the return value as far as I can see so this is confusing

In check_aces instead no value is returned but the name is very similar to the function above where something is returned.

You should try to keep consistent language between your methods.

Built-in count and list comprehension

You say Check if 'A' in Hand but you do not use the in keyword!

This way is much simpler:

figures = [card.figure for card in self.cards]
self.aces = figures.count('A')

restart

Conceptually I feel like

def restart(self):
    self.deck = [Card(suit, figure) for suit in SUITS for figure in FIGURES]

should shuffle the deck, you cannot restart with all the cards in order.

"Native" iteration

    for i in range(len(self.cards)):
        self.value += self.cards[i].value

Becomes:

for card in self.cards:
    self.value += card.value

Much cleaner.

This page is good to learn python built-in methods for clean looping https://www.datadependence.com/2016/02/pythonic-for-loops/

Potential debug problems

    while True:
        try:
            bet = int(input('Tell me your bet (should be lower or equal then your money)'))
        except:
            continue

Always specify which exception to except otherwise this suppresses all kinds of errors and can make debugging inconvenient.

minor

def check_money(self):
    self.play = self.money > self.min_bet

Good

I see you separated logic and input/output handling, this is very good.

Answer 2

Bugs

• Do you want to play? infinite loop after pressing a wrong key, the program keeps printing the same prompt over and over no matter what the input is.
• Negative inputs: The program accepts negative monetary values and adds them to your current wealth if you lost.

Style

I suggest you check PEP0008 https://www.python.org/dev/peps/pep-0008/ the official Python style guide when writing your code and Flake8 http://flake8.pycqa.org/en/latest/ as a tool for style enforcement and the following goes accordingly:

• Missing whitespace after a comma:

  SUITS = ['Heart','Diamond','Club','Pike']
  FIGURES = ['A','2','3','4','5','6','7','8','9','10','J','Q','K']
  def __init__(self,suit,figure):
  elif self.figure in ['J','Q','K']:
  

  are written:

  SUITS = ['Heart', 'Diamond', 'Club', 'Pike']
  FIGURES = ['A', '2', '3', '4', '5', '6', '7', '8', '9', '10', 'J', 'Q', 'K']
  def __init__(self, suit, figure):
  elif self.figure in ['J', 'Q', 'K']:
  

• Docstrings:

  '''
  Single card class
  Use it in loop to make full french deck of card
  print return 'Figure of suit'
  '''
  

  Python documentation strings (or docstrings) provide a convenient way of associating documentation with Python modules, functions, classes, and methods. It's specified in source code that is used, like a comment, to document a specific segment of code and are usually accessed using help() They should describe what the classes/functions do instead of how and are delimited by triple double quotes. Most of your defined methods do not contain docstrings while they should.

  The docstring above should be enclosed in triple double quotes and should contain a description of what the class does not how:

  """
  Build a french deck of cards.
  """
  

• Blank lines: Surround top-level function and class definitions with two blank lines.Method definitions inside a class are surrounded by a single blank line.Extra blank lines may be used (sparingly) to separate groups of related functions. Blank lines may be omitted between a bunch of related one-liners (e.g. a set of dummy implementations).

  Examples:

  2 lines should be left between these:

  MIN_BET = 2
  
  class Card:
  

  And these:

  self.deck = [Card(suit, figure) for suit in SUITS for figure in FIGURES]
  
  class Hand:
  

  And too many blank lines are left in your main() function.

• Bare except: You should be indicating the type of exception likely to occur (ex: IndexError, TypeError ...) instead of the too broad exception clause.

• Typo: def deposite(): is def deposit():

• Comments: start with a single # and a capital letter and end with .

  ##Creating Objects is written # Creating objects.

  and

  ##1 round -> Draw, hit/stand -> dealer -> check if win

  is written:

  # 1 round -> Draw, hit/stand -> dealer -> check if win. and same goes for all other comments

Code

• Global variables: are bad in Python and all programming languages, the reason behind them being bad that they might produce some very hard to trace side effects and lead something called 'Spaghetti Code'. I suggest you enclose them inside their respective functions/methods that use them or you might even make them class variables (if that's necessary).

• inefficient pop(): In your Deck class:

  def draw(self):
  return self.deck.pop(0)
  

  when you're popping the first element of a list there will be n - 1 rearrangements of the remaining items in the list which is inefficient for large lists(not the case here). And since the deck will be shuffled so only return self.deck.pop() is sufficient.

• Instance attributes defined outside constructor: In your check_value() method:

  if self.value > 21:
      self.isBusted = True
  

  instance attributes should be defined inside the constructor __init__() only.

• Magic numbers:

  self.value = 11
  self.value = 10
  while self.value > 21 and ace > 0:
  self.value -= 10
  while self.value < 17:
  self.pool = 0
  

  These numbers should be function parameters or have some commented explanations provided.

Answer 3

Looks like a pretty good start. Your Deck and Card classes are basically fine, aside from perhaps a few docstrings. I should be telling you to really add them, and it's definitely better practice, but on the other hand they're clear and readable enough that you can do without them.

Only minor details is those you could keep rename shuffle to _shuffle and call it everytime in __init__ and restart, but that's just different code, not neccesarily better. It's advantage would be that you can't forget to call it.

Hand

Hand.check_aces()

Why is there a check_aces() method? You use it only once. In fact, I daresay you could make the check a simple one-liner with a generator expression:

    def check_aces(self):
        return sum(card.figure == "A" for card in self.cards) 

This checks all the cards in self.cards, and returns a boolean True for each ace. Since the boolean True has an integer value of 1, you can sum them to count the aces.

You could easily inline it like that one-liner, but we'll leave it as a function for now, for the sake of clarity.

Do please note that I return the value instead of just calculating and storing it. Bear with me for a moment, you'll see why.

Hand.check_value()

Your docstring is wrong, you do return a value. And that's generally a good thing. However, given that you never use it, it might be all-right in this case. For the chase of perfection, however, you could make this a cached calculation instead, which is slightly easier to use then what you do now, but without calculating it more than required. This would look something like this:

    def __init__(self):
        self._value = 0  # Note the rename
        self.cards = []

    @property
    def value(self):
        if not self._value:  # Same as "is zero"
            aces = check_aces()  
            # This is the only place we actually use the number of aces. That makes it better to 
            # just return it instead of using two lines - one to calculate and one to retrieve.
            value = sum(card.value for card in self.cards)
            # Generator expression in a sum again. Most simple loops can be replaced this way.
            if aces > 1:
                value -= (aces - 1) * 10
            self._value = value
        return self._value

If you aren't familiar with @property, here are the docs

This will first check if the saved value is zero. If so, it will calculate the value, then return it. And this is all done the moment someone else types hand.value(). But if it is not zero, then it recalculates it.

Do note that I removed the isBroken variable. You never used it.

Normally in cache tracking you'd have a different variable as a flag to see if you need to refresh your cache, instead of a magic value that we use here. Or they use timestamps. However, this is small enough in scope that I think a magic variable isn't harmful. Do note, however, that you need to signal this recalculation every time the hand value changes, therefore:

def add_card(self, card):
    self.cards.append(card)
    self._value = 0

def restart(self):
    self.cards = []
    self._value = 0

Hand.dealer_ai()

    def dealer_ai(self,d):
        while self.value < 17:
            self.add_card(d.draw())
        print(f'Dealer cards: {self.cards} ')

Due to the check_value refactoring we did by shoving it into a property, note that we can just remove 2 different calls to that function here, making it that much easier to read. However, this function really does two things - it acts on it's state, which is very appropriate to the function name, and it prints it. That's a lot less appropriate. You should also return this value, or just leave it and reach into the class and retrieve Hand.cards from outside when you want to use this.

Bank

Back.check_money()

Same as the check_value, really. Make it a property, like this:

    @property
    def play(self):
        return self.money < self.min_bet

I'd cache here to, but comparing two integers is so check that no caching will ever be an advantage. Do note that the "<" comparison operator, along with all other operators you generally use in an if (expression): block, already return boolean values. Therefore you can return it directly. The only exceptions are the or and and operators, which will return the last evaluated value instead. (which has it's own uses I won't go into now.)

Bank.bet()

This really should be a method that takes an integer. It should not check input values. And it should certainly not recurse on invalid values. The print()s here also should really be handled by retrieval from outside.

You also forgot to check for your value being bigger than min_bet.

    def bet(self, value):
        """ Bets money. Assumes there are valid bets possible. """
        if self.min_bet <= value <= self.money:
            self.pool = 2*bet
            self.money -= bet
            return True  # Tell the caller the Bank is ok with this bet.
        return False  # Tell the caller the bet is rejected.

If you want input validation, you can write a function for that:

def get_integer(request_message):
    while True:
        try:
            return int(input(request_message))
        except ValueError:
            continue

And then use it like this:

value = 0
while bank.play:  # Checks if there are valid bets
    value = get_integer("Tell me your bet (should be lower or equal then your money)")
    if bank.bet(value):
        break
    else:
        print(f"Sorry, {value} is not a valid bet. It has to be between {bank.min_bet} and {bank.money}.")

Bank.player_[win/lose]

Again, move the prints out. Looks good for the rest.

main

You can drop the entire while True: loop. You never continue, only break, and with that last if-statement, you always break the first iteration.

If you follow the @property recommendations above, you can of course drop all check_value() statements in the loop, so I won't explicitly point them out. Instead I'll silently drop them.

            choice = input('\n\nWhats your choice? - "h" for hit / "s" for stand')
            while choice.lower() == 'h':
                player_hand.add_card(deck.draw())
                print(f'\n\nYour cards: {player_hand.cards} with {player_hand.value} value')
                if player_hand.value >= 21:
                    break
                choice = input('\n\nWhats your choice? - "h" for hit / "s" for stand')

You're duplicating the input() function call here. Better do:

            while 'h' == input('\n\nWhats your choice? - "h" for hit / "s" for stand').lower().strip():
                player_hand.add_card(deck.draw())
                print(f'\n\nYour cards: {player_hand.cards} with {player_hand.value} value')
                if player_hand.value >= 21:
                    break

Which is shorter and more readable. Readability is also the reason I switched the "h" and the input call - you can see what happens even if you don't read the entire line.

Victory Check

I would just call the functions directly, instead of grabbing a manual variable.

            if player_hand.value == 21:
                bank.player_win()
            elif player_hand.value > 21:
                bank.player_lose()

And of course the same with the others. Note that the removal of the while True: loop also allows us to skip all those break statements, at the low cost of changing all ifs that aren't the first to elifs instead. Alternatively, you can structure it like so:

            dealer_hand.dealer_ai(deck)
            if player_hand.value == 21 or dealer_hand > 21 or player_hand.value > dealer_hand.value:
                bank.player_win()
            else:
                bank.player_lose()

(Note: It took me a while to write, and there are 2 new answers since I started... I haven't checked them, though.)