I'm new to programming and have been teaching myself to code for about 6 months. Since I have no one to ask about any of coding-related questions, I may have developed bad habits in the meantime, but I can't tell myself. As such, I wrote a simple blackjack game in Python, to demonstrate how I usually code, and to find out if there is something I should've done (or I shouldn't have done).

This is the start of the code.

from random import shuffle, uniform
from os import system, name, get_terminal_size as tsize
from time import sleep

This is Card class. I don't like display() method, because it's too ugly (especially with some fonts that breaks the shape). But I'm not sure how to improve this. I'm also not sure where should I place sdict and color. Currently, it's a class variable. Should I pull these out of Card class and put it on top? And I don't know when exactly I should implement those getters/setters. I just make them every time. Is this a bad practice? Lastly, uid is doing nothing currently but I put it there just in case. Should I remove such useless things on main branch and only leave it on test/dev branch?

class Card(object):
    sdict = {'S': '♤', 'H': '♡', 'D': '♢', 'C': '♧'}
    color = {'r': '\033[1;31;49m', 'w': '\033[1;37;49m'}

    def __init__(self, suit, rank, uid):
        self.suit = suit
        self.symbol = Card.sdict[suit[0]]
        self.rank = rank
        self.uid = uid
        self.is_red = self.suit[0] in 'HD'
        self.value = 10 + (rank[0] == 'A') if rank.isalpha() else int(rank)

    def __str__(self):
        r = self.get_rank()
        if self.is_red:
            return Card.color['r'] + self.symbol + r + Card.color['w']
        return self.symbol + r

    def get_suit(self, colored=0) -> str:
        if colored and self.is_red:
            return Card.color['r'] + self.suit + Card.color['w']
        return self.suit

    def get_symbol(self, colored=0) -> str:
        if colored and self.is_red:
            return Card.color['r'] + self.symbol + Card.color['w']
        return self.symbol

    def get_rank(self, colored=0) -> str:
        r = self.rank[0] if self.rank != '10' else self.rank
        if colored and self.is_red:
            return Card.color['r'] + r + Card.color['w']
        return r

    def get_uid(self) -> int:
        return self.uid

    def get_value(self) -> int:
        return self.value

    def display(self, hide=0) -> list[str]:
        if hide:
            lines = ['┏━━━━━━━━━━━┓' if not i else
                     '┃███████████┃' if i != 10 else
                     '┗━━━━━━━━━━━┛' for i in range(11)]
            return lines
        pad = ' ' * (self.get_rank() != '10')
        is_alpha = self.get_rank().isalpha()
        sym, val = self.get_symbol(1) + ' ', self.get_value()
        mid_cen = (sym * (val in (1, 3, 5, 9) or is_alpha)).ljust(2)
        midside = (sym * (5 < val < 9)).ljust(2)
        rdiff_1 = (sym * (val in (9, 10) and not is_alpha)).ljust(2)
        row_2up = (sym * (val in (7, 8, 10) and not is_alpha)).ljust(2)
        row_2dn = (sym * (val in (8, 10) and not is_alpha)).ljust(2)
        corners = (sym * (val > 3 and not is_alpha)).ljust(2)
        top_bot = (sym * (val in (2, 3))).ljust(2)
        lines = [
            '┃{}         ┃'.format(self.get_rank(1) + pad),
            '┃  {} {} {} ┃'.format(corners, top_bot, corners),
            '┃     {}    ┃'.format(row_2up),
            '┃  {}    {} ┃'.format(rdiff_1, rdiff_1),
            '┃  {} {} {} ┃'.format(midside, mid_cen, midside),
            '┃  {}    {} ┃'.format(rdiff_1, rdiff_1),
            '┃     {}    ┃'.format(row_2dn),
            '┃  {} {} {} ┃'.format(corners, top_bot, corners),
            '┃         {}┃'.format(pad + self.get_rank(1)),
        return lines

This is Shoe class, which doesn't do much. I could just handle the job on Blackjack class, or merge this into Card class, or separate the job into many classes. I'm not sure which task deserves its own class. And should I avoid using the print on __init__? Also, is there any way to use type hint with the default parameter value (like num_decks=6 and vb=False)?

class Shoe(object):
    def __init__(self, num_decks=6, vb=False):
        print(f'Generating a dealing shoe with {num_decks} decks', end='')
        self.shoe = self.build_shoe(num_decks)

        print('\nShuffling the cards...' if vb else '', end='')

        print('\nGenerating a random cut' if vb else '', end='')
        self.shoe = self.cut_deck(self.shoe)
        print(f'\nRemoved {52 * num_decks - len(self.shoe)} cards '
              f'({len(self.shoe)} cards remaining)' if vb else '', end='')

    def build_shoe(num_decks: int) -> list[Card]:
        suits = 'Spade Heart Diamond Club'.split()
        ranks = 'Ace 2 3 4 5 6 7 8 9 10 Jack Queen King'.split()
        shoe = []
        for _ in range(num_decks):
            for suit in suits:
                for rank in ranks:
                    shoe.append(Card(suit, rank, len(shoe)))
        return shoe

    def cut_deck(shoe: list[Card], minimum=0.30, maximum=0.50) -> list[Card]:
        cut = uniform(minimum, maximum)
        del shoe[:int(len(shoe) * cut)]
        return shoe

    def get_shoe(self) -> list[Card]:
        return self.shoe

    def get_length(self) -> int:
        return len(self.shoe)

    def deal(self, num_cards=1) -> list[Card]:
        cards = []
        for _ in range(num_cards):
        return cards

This is Hand class. Should I always try to reduce the number of iterations? For example, self.total_value, self.num_aces and self.hand_value can be calculated in one sweep, but it will result in vertically longer code (though shorter horizontally). Even if the expected number of iterations is low, should I try to reduce it? Also, should I strictly limit the number of characters to 79? or is it merely a suggestion?

class Hand(object):
    def __init__(self, hand=None):
        self.hand = [] if hand is None else hand
        self.total_value = sum(c.get_value() for c in self.hand) if self.hand else 0
        self.num_aces = sum(c.get_value() == 11 for c in self.hand) if self.hand else 0
        self.hand_value = 0
        self.decision = ''

    def add_cards(self, cards: list[Card]) -> None:
        for card in cards:
            self.total_value += (v := card.get_value())
            self.num_aces += 1 if v == 11 else 0

    def get_hand(self) -> list[Card]:
        return self.hand

    def get_printable(self, reveal=22) -> list[str]:
        cards = [c.display() if reveal > i else c.display(hide=True)
                 for i, c in enumerate(self.hand)]
        return ['   '.join(c[i] for c in cards) for i in range(len(cards[0]))]

    def get_hand_value(self) -> int:
        return self.hand_value

    def set_hand_value(self) -> None:
        if self.total_value <= 21:
            self.hand_value = self.total_value
        t, n = self.total_value, self.num_aces
        while n and t > 21:
            n -= 1
            t -= 10
        self.hand_value = t

    def get_decision(self) -> str:
        return self.decision

    def set_decision(self, decision: str) -> None:
        self.decision = decision

    def length(self) -> int:
        return len(self.hand)

    def is_busted(self) -> bool:
        return self.get_hand_value() > 21

    def is_blackjack(self) -> bool:
        return self.length() == 2 and self.hand_value == 21

    def is_playable(self) -> bool:
        return not (self.get_hand_value() >= 21 or self.decision in ('s', 'dd'))

    def is_splittable(self) -> bool:
        return self.length() == 2 and self.hand[0].get_value() == self.hand[1].get_value()

and lastly, Blackjack class and if __name__ == '__main__': thingy. It is very messy but I always ended up like this somehow. Maybe it's because I usually don't plan anything ahead and just dive right into coding, but then again, I don't know what exactly I should plan ahead.

class Blackjack(object):
    def __init__(self):
        self.nt = name == 'nt'
        print('\033[1;37;49mWelcome to the game of Blackjack', end='')
        self.shoe = self.generate_shoe()
        self.bankroll = self.get_int_input(
            f'\nEnter your bankroll [$100-$100m] (Default: ${50_000:,}): ',
            int(1e8), 100, 50_000)
        self.min_bet = max(5, self.bankroll // 1000 // 1000 * 1000)
        self.dealer = Hand()
        self.player = [Hand()]
        self.bet = 0
        self.initial_bet = 0
        self.num_round = 0

    def generate_shoe(self) -> Shoe:
        d = 6
        msg = f'\nEnter the number of decks [2-12] (Default: {d}): '
        return Shoe(self.get_int_input(msg, 12, 2, d), vb=True)

    def get_int_input(msg: str, hi=6, lo=2, default=6) -> int:
        user_input = lo - 1
        while user_input < lo or user_input > hi:
                tmp = input(msg)
                user_input = int(tmp) if tmp != '' else default
                if not (lo <= user_input <= hi):
                    print('Input out of range. Please try again.')
            except ValueError:
                print('Invalid input. Please try again.')
        return user_input

    def get_bool_input(msg: str, opt='YN') -> bool:
        user_input = '1'
        while user_input.upper()[0] not in opt:
            user_input = input(msg)
            if user_input == '':
                user_input = opt[0]
        return user_input.upper().startswith('Y')

    def take_decision(hand: Hand, msg: str, possibles: list[str]) -> None:
        decision = ''
        if hand.length() > 2:
            del possibles[2:]
        elif not hand.is_splittable():
            del possibles[3:]
        opt = '/'.join(c if i else c.upper() for i, c in enumerate(possibles))
        while decision[:2].lower() not in possibles:
            decision = input(msg + '[' + opt + ']: ')
            decision = possibles[0] if decision == '' else decision

    def take_bet(self) -> None:
        d = max(self.min_bet, self.bankroll // 1000 // 1000 * 10)
        msg = f'Place your bet (Default: ${d}): '
        self.bet = self.get_int_input(msg, hi=self.bankroll, lo=self.min_bet, default=d)
        self.initial_bet = self.bet
        self.bankroll -= self.bet

    def get_insurance_premium(self) -> int:
        premium = 0
        if self.dealer.get_hand()[0].get_rank().startswith('A'):
            if self.get_bool_input('Do you want to take an insurance [y/N]? ', 'NY'):
                premium = self.initial_bet // 2
                self.bankroll -= premium
        return premium

    def print_board(self, rv=22) -> None:
        width = tsize()[0]
        print(f'Bet: {self.bet:,}'.ljust((w := width // 3))
              + f'Round {self.num_round}'.center(w)
              + f'Bankroll: {self.bankroll:,}'.rjust(w))

              + (rv > 0) * f': {" ".join([str(c) for c in self.dealer.get_hand()][:rv])}'
              + (rv > 1) * f' ({self.dealer.get_hand_value()})')
        print('\n'.join([line for line in self.dealer.get_printable(rv)]))

        for i, h in enumerate(self.player):
            n = f'{(" (Hand " + str(i + 1) + ")") * (len(self.player) > 1)}'
            print(f'\nPlayer{n}: {" ".join([str(card) for card in h.get_hand()])}'
                  f' ({h.get_hand_value()})')
            print('\n'.join([line for line in h.get_printable()]))

    def get_payout(self) -> int:
        payout, dv, lp = 0, self.dealer.get_hand_value(), len(self.player)
        for i, h in enumerate(self.player):
            n = f'Hand {i + 1}' if lp > 1 else 'You'
            hv, dd = h.get_hand_value(), 1 + (1 * (h.get_decision() == 'dd'))
            if h.is_busted() or hv < dv <= 21:
                print(f'{n} lose')
            if dv > 21 or hv > dv:
                print('Dealer busted. ' * (dv > 21) + f'{n} win')
                payout += self.initial_bet * (2.5 if h.is_blackjack() else 2) * dd
            elif dv == hv:
                payout += self.initial_bet * dd
        return payout

    def cls(self) -> None:
        if self.nt:

    def round(self) -> None:
        self.player = [Hand(self.shoe.deal(2))]
        if not self.player[0].is_blackjack():
            while any(h.is_playable() for h in self.player):
                for i, hand in enumerate(self.player):
                    if not hand.is_playable():
                    s = f'(Hand {i + 1}) ' * (len(self.player) > 1)
                    self.take_decision(hand, f'Enter your decision {s}', 'h s dd sp'.split())
                    if (d := hand.get_decision()) != 's':
                        self.bankroll -= self.initial_bet if d in ('dd', 'sp') else 0
                        self.bet += self.initial_bet if d in ('dd', 'sp') else 0
                        if d == 'sp':
                            self.player += [Hand([hand.get_hand()[0]]), Hand([hand.get_hand()[1]])]
                            del self.player[i]

        in_play = not all(h.is_busted() for h in self.player)
        premium = self.get_insurance_premium() if in_play else 0

        while self.dealer.get_hand_value() < 17 and in_play:

        payout = self.get_payout()
        self.bankroll += payout
        print(f'You received a payout of ${payout}')
        if premium and self.dealer.get_hand()[1].get_value() == 10:
            self.bankroll += premium * 2
            print(f'You received an insurance payout of ${premium * 2}')

    def play(self) -> None:
        more = True
        while more and self.bankroll >= self.min_bet:
            self.num_round += 1
            more = self.get_bool_input('Do you want to play more rounds [Y/n]? ', 'YN')
            if (ls := self.shoe.get_length()) < 15:
                msg = f"Only {ls} cards left in dealer's shoe. "
                msg += "Do you want to replenish the shoe [Y/n]? "
                if self.get_bool_input(msg, 'YN'):
                    self.shoe = self.generate_shoe()
                    print("Game Over (too few cards remaining)")

if __name__ == '__main__':

Feel free to tell me anything that I could improve my code, style, etc, and anything I should, or shouldn't have done. Lastly, I'd like to know what kind of speed I should aim for. This took some hours for me to code (1 < h < 10). Should I aim for < 60 minutes? or < 30 mins? Thank you in advance.

EDIT: this is a link to the most recent version of my blackjack code for anyone who is interested.

  • \$\begingroup\$ is the program exclusive to unix runtime? \$\endgroup\$
    – hjpotter92
    Oct 27, 2021 at 3:49
  • \$\begingroup\$ @hjpotter92 No. AFAIK, it works everywhere. But it may not run on vanilla PyCharm due to os.system() and os.get_terminal_size(). \$\endgroup\$
    – e1630m
    Oct 27, 2021 at 6:41
  • \$\begingroup\$ it won't work on windows/powershell too, due to the color code and symbols. \$\endgroup\$
    – hjpotter92
    Oct 27, 2021 at 6:55
  • \$\begingroup\$ Strange because I've tested on 3 different Windows environments. 1 2 3 \$\endgroup\$
    – e1630m
    Oct 27, 2021 at 7:25
  • \$\begingroup\$ i.sstatic.net/pq8XA.png this is in the plain powershell provided and is not the fluent based windows terminal \$\endgroup\$
    – hjpotter92
    Oct 27, 2021 at 8:47

2 Answers 2


Planning ahead

It is very messy but I always ended up like this somehow. Maybe it's because I usually don't plan anything ahead and just dive right into coding, but then again, I don't know what exactly I should plan ahead.

I recommend to work in baby steps. Think of the next valuable feature, in its simplest form, and implement just that. When you are done with a baby step, pause and take a step back, review what you did considering the bigger picture of your ultimate goal, and verify that you're still going in the right direction, or else revert.

For example here, I would first implement the most basic game possible, without many of the features you currently have:

  • no fancy output, no colors
  • no shoe feature
  • play just one game
  • implement the essential rules only, without payout, insurance, etc
  • ... and so on ...

If that had been your first post, reviewers could have focused on the high level design, and could have provided early guidance to prevent going down paths that are not very useful and time consuming.

Keeping things simple

Every piece of code is a burden. It has to be understood and maintained. Therefore it's important that every piece of code is useful.

If you don't fully understand why something is there, I suggest to delete it. If the program doesn't work without it, try to understand why.

uid is doing nothing currently but I put it there just in case.

Drop it. It's not useful. The reader has to understand it, and in fact there's nothing to understand, because it really has no purpose right now. It's a friction for the reader at every occurrence of uid in this program. For example:

  • When reading the constructor of Card, I'm wondering what is this for.
  • When trying to create a Card, I'm wondering what value I should pass, and what will be the consequences.

If it's not there, I save many unnecessary mental cycles.

Remove anything that is not used. For example Card.get_suit.

Another example is the colored parameter of get_symbol: the function is only ever called with colored=1, so the parameter is not useful.

Do this recursively, after you removed something, check what else has become unused. For example, after removing Card.get_suit, it turns out that self.suit is also not used.

If you had built up the code from scratch only adding what is absolutely necessary in each step, then there would be less to remove now, less code to review and to understand, therefore less mental burden.

Card.display is extremely hard to understand and to maintain. If there was a separate template for each card, that would be easy to understand and to maintain, even if that will make the program longer, it would be a good tradeoff.

Optimize code for reading, first. If there are problems with that, fix it in a later step, if really important.

Avoid recomputing values that are not supposed to change

For example Card.get_symbol will recompute the symbol on every call, based on values that never change after the card is created. It would be better to compute this only once, at the time the card is created. The same goes for Card.get_rank, Card.__str__.

Avoid duplicated logic

This code pattern appears in many places in Card:

Card.color['r'] + something + Card.color['w']

It would be better to extract this logic to a common function with a descriptive name. It will make it easier to understand the code, and if you need adjust the logic in the future, you will be able to do it in one place.

Hide implementation details

Hand.set_hand_value is only used internally within Hand. It should not never be called from outside. To signal this to readers, rename with a single underscore _ prefix.

Consider real life objects to guide your design

Hand.add_cards looks strange, because the name is plural. When in a game of Blackjack do we ever add more than one card to the hand? I think never, so this is confusing.

An add_card function that takes a single Card parameter would be natural.

Other technical Python coding issues

Several methods in Card take a colored parameter with values 0 or 1, and used in boolean context. The parameter should be a proper boolean.

The if self.hand else 0 is unnecessary:

self.hand = [] if hand is None else hand
self.total_value = sum(c.get_value() for c in self.hand) if self.hand else 0
self.num_aces = sum(c.get_value() == 11 for c in self.hand) if self.hand else 0

You asked if it would be better to make only one pass over self.hand, and compute total_value and num_aces in that loop. Yes, one pass is usually better than 2.

Instead of the many get_ methods, consider using properties.

  • \$\begingroup\$ Hand.add_cards: I'm not familiar with blackjack, and I thought they initially deal 2 cards at once and then start to deal one card (just like I did on Blackjack.round). After watching a few blackjack tournament videos, I realized that's not the case. \$\endgroup\$
    – e1630m
    Nov 5, 2021 at 23:44
  • \$\begingroup\$ if self.hand else 0: I put it there because sum(None) will break when hand is None (this happens on Blackjack.__init__). I could just start with something like hand = [] if hand is None else hand and remove if else for those three variables, or change self.dealer and self.player on Blackjack.__init__ to be initialized with a dummy card, or remove the self.dealer and self.player from Blackjack.__init__ altogether and call new instance of Hand with a card every round. I'm going to modify my code for the last option. \$\endgroup\$
    – e1630m
    Nov 5, 2021 at 23:46
  • \$\begingroup\$ @e1630m self.hand = [] if hand is None else hand makes sure that self.hand is not None, and that's why you don't need the if self.hand else 0 on the next two lines \$\endgroup\$
    – janos
    Nov 6, 2021 at 5:31
  • \$\begingroup\$ Oops. I mistakenly wrote the comment after checking the slightly modified version that does the job in one sweep but iterates over hand not self.hand. \$\endgroup\$
    – e1630m
    Nov 6, 2021 at 7:47
  • Don't inherit from (object); Python 2 needs to die (and you already rely on Python 3 given your current code)
  • An OO approach would have a Suit class (even if it's a lightweight named tuple) with properties of, perhaps: symbol, name, letter, colour. This is a little less awkward than is_red. These properties can then be factored away from Card, and only a reference to a suit object kept for each card.
  • Having the color dictionary on Card doesn't make much sense; you'll want to have a module or class that takes care of screen operations such as clear, and ANSI escape sequences for colour etc.
  • The pattern Card.color['r'] + self.symbol + r + Card.color['w'] calls out for a utility or two. One pattern you could follow is a @contextmanager that sets the colour, runs the inner code, and then clears the colour after (even if an exception is thrown).
  • You have some typehints - good! But they're inconsistent. For instance,
    def get_symbol(self, colored=0) -> str:

needs a hint for colored. Since you asked, your hand=None should be hand: Optional[List[Card]] = None.

  • Your Blackjack constructor shouldn't be responsible for running the entire game; just initialisation.
  • The call self.dealer.__init__() is concerning. Presumably that object is already initialised via Hand(), so - calling it again is surprising. If you just want to replace the instance, replace the instance.
  • length, is_busted etc. are a good fit to be @propertys. The purpose of a property is not particularly to do a character-by-character code length trade; it's to communicate to callers of your class that certain values from your class are available without needing to pass parameters, ideally without mutating the class, and without a whole lot of computation. Properties are also convenient when using modern debuggers as they appear without having to be explicitly called.
  • You have some manually-formatted currency strings. locale.currency is made for this.
  • self.bankroll // 1000 // 1000 * 1000 seems like you're doing a little bit of trickery to get a round effect. That's less obvious than just calling round. You say I was trying to give two different min_bet for users. $5 for someone with less than $1 million bankroll, and roughly 0.1% of bankroll rounded down to a thousand dollars for someone with a million or above bankroll. Clear code for this behaviour is important (also, don't name a variable d):
if self.bankroll < 1e6:
    default = self.min_bet
    default = round(self.bankroll/1000, -3)
  • user_input < lo or user_input > hi is equivalent to not (lo <= user_input <= hi)
  • Apparently this is controversial, but my opinion is that $100m is ten cents. $100M is 100 million dollars.
  • \$\begingroup\$ (object): Thanks. I didn't know (object) was a Python 2 thing. I saw some classes have these bits, while some don't, and started to put it everywhere without any understanding. \$\endgroup\$
    – e1630m
    Nov 5, 2021 at 23:34
  • \$\begingroup\$ Type hints: I didn't know how to use type hints with default parameters. Hence, colored=0 as well as opt='YN', rv=22, hi=6, minimum=0.30, etc. Now I figured it out how to use it (for example, opt: str = 'YN'), but still Hand.__init__ kinda bugs me, since hand: list = None looks somewhat weird to me. \$\endgroup\$
    – e1630m
    Nov 5, 2021 at 23:34
  • \$\begingroup\$ @property decorator: Is there a reason to use this? It would add one line for each is_something() and remove two characters () when I call it, which doesn't sound like a good trade to me. If I replace getters, I would earn 4 more spaces on each time I call it (by removing get_), but still doesn't look like a sweet deal. Is it primarily used to narrow down multiple options (to access attribute) to one? Or is there a way to merge multiple getters into one universal getter? \$\endgroup\$
    – e1630m
    Nov 6, 2021 at 0:05
  • \$\begingroup\$ self.bankroll // 1000 // 1000 * 1000: I was trying to give two different min_bet for users. $5 for someone with less than $1 million bankroll, and roughly 0.1% of bankroll rounded down to a thousand dollars for someone with a million or above bankroll. I could achieve the same outcome with __import__('math').floor(self.bankroll // 1e6 * 1e3) or int(self.bankroll // 1e6 * 1e3), but opted for that one. Maybe I should just settle for universal 0.1% minimum bet with an absolute minimum of $5 (something like min(5, round(self.bet // 1000)). \$\endgroup\$
    – e1630m
    Nov 6, 2021 at 0:34
  • \$\begingroup\$ [it] doesn't sound like a good trade to me - because counting individual characters like that is not really the overriding criterion. I've edited my answer with a little more explanation. Is it primarily used to narrow down multiple options (to access attribute) to one? Or is there a way to merge multiple getters into one universal getter? No, and 'technically yes but it would be more work than benefit'. \$\endgroup\$
    – Reinderien
    Nov 6, 2021 at 12:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.