5
\$\begingroup\$

I just started learning python a couple of weeks back from and like many others here, I too seek feedback on my simplified Blackjack game. I would like to know if the code could have been more elegant/ shorter and how.

PS - In my code, aces count only as 1 for the time being.

class Bank():

    def __init__(self,bet,balance):
        self.bet = bet
        self.balance = balance

    def transaction_win(self):
        self.balance+=self.bet*2
        return self.balance

    def transaction_loss(self):
        self.balance-=self.bet
        return self.balance

def introduction():
    print('*'*40)
    print('\nWelcome to Blackjack!')
    print('This is a simplified version of the actual game')
    print('Each card club is represented by its initial i.e. (H)earts,(S)pade,(D)iamond and (C)lub')
    print('Player can only Hit or Stay. Double down, insurance, card splits are not a feature of the game')
    print('Rest of the rules remain the same. Good luck and have fun!\n')
    print('*'*40)
    print('\n\n')
    balance = int(input('What is the maximum total balance you can bet?'))
    bet = int(input('What amount would you like to bet?'))
    return balance,bet

class Deck():

    def __init__(self,card_no):
        self.card_no = card_no

    def card_display(self):
        if self.card_no in range(1,14):
            card_house = 'H'
        elif self.card_no in range(14,27):
            card_house = 'S'
        elif self.card_no in range(27,40):
            card_house = 'D'
        else:
            card_house = 'C'
        temp = self.card_no%13
        if temp==0:
            temp = 13
        return str(temp)+card_house

    def card_score(self):
        temp = self.card_no%13
        if temp in range(1,11):
            card_score = 1
        elif temp in range(11,13):
            card_score =  10
        else:
            card_score = 10
        return card_score

def start_game(player_hand,dealer_hand):
    cards = list(range(1,53))
    shuffle(cards)
    print('Card deck shuffled!')
    print('Dealing cards\n\n')
    player_card = [cards.pop(),cards.pop()]
    dealer_card = [cards.pop(),cards.pop()]
    player_score = []
    dealer_score = []

    for (i,j) in zip(player_card,dealer_card):
        temp1 = Deck(i)
        temp2 = Deck(j)
        player_hand.append(temp1.card_display())
        player_score.append(temp1.card_score())
        dealer_hand.append(temp2.card_display())
        dealer_score.append(temp2.card_score())

    return player_hand,dealer_hand,cards,player_score,dealer_score

def hand_score(player_score,dealer_score):
    return sum(player_score),sum(dealer_score)

def display_hand(player_hand,dealer_hand,choice):

    if choice == 1:
        print('Dealer hand:',end=' ')
        for i in dealer_hand[:-1]:
            print(i,end=' ')
        print('*')
    else:
        print('Dealer hand: '+' '.join(dealer_hand))
    print('\n'*5)
    print('Player hand: '+' '.join(player_hand))

def player_choice():
    while True:
        try:
            choice = int(input('Do you want to Hit(1) or Stay(2)?'))
        except:
            print('Error! Not a numeral input or invalid numeral input. Please try again')
        else:
            if choice not in [1,2]:
                print('Invalid numeral input. Please try again')
                continue
            else:
                return choice
                break


def win_check(player_total,dealer_total,bet,balance,choice):
    temp = Bank(bet,balance)
    if player_total == 21:
        print('Congrats! You won!')
        balance = temp.transaction_win()
        return True,balance
    elif dealer_total == 21 and choice == 2:
        print('Sorry, you lost your bet')
        balance = temp.transaction_loss()
        return True,balance
    elif player_total>21:
        print('Bust! Sorry, you lost your bet')
        balance = temp.transaction_loss()
        return True,balance
    elif dealer_total<21 and dealer_total>player_total and choice == 2:
        print('Sorry, you lost your bet')
        balance = temp.transaction_loss()
        return True,balance
    elif dealer_total>21 and choice == 2:
        print('Congrats! You won!')
        balance = temp.transaction_win()
        return True,balance
    else:
        return False,balance

def play_again(balance):
    wish = input('Do you wish to play again? (Y)es or (N)o?')
    bet = 0
    if 'y' in wish.lower():
        answer = True
        print('Available balance: {}'.format(balance))
        while True:
            try:
                bet = int(input('how much would you like to bet?'))
            except:
                print('please give a numeric input')
            else:
                if balance-bet<0:
                    print('Please do not bet more than available balance. Try again')
                    continue
                else:
                    break
    elif 'n' in wish.lower():
        answer = False
        print('Thanks for playing. Your final balance is {}'.format(balance))
    else:
        clear_output()
        print('Give proper input please')
        play_again(balance)
    return answer,bet

from random import shuffle
from IPython.display import clear_output

play_answer = True
balance,bet = introduction()

while balance>0 and play_answer == True:

    choice = 1
    winner_check = False
    player_hand,dealer_hand,cards,player_score,dealer_score = start_game([],[])
    player_total,dealer_total = hand_score(player_score,dealer_score)
    display_hand(player_hand,dealer_hand,choice)
    choice = player_choice()
    while (choice == 1 or choice == 2) and winner_check == False:

        card_drawn = cards.pop()
        temp = Deck(card_drawn)
        if choice == 1:
            player_total+=temp.card_score()
            player_hand.append(temp.card_display())
            player_score.append(temp.card_score())
        else:
            dealer_total+=temp.card_score()
            dealer_hand.append(temp.card_display())
            dealer_score.append(temp.card_score())

        clear_output()
        display_hand(player_hand,dealer_hand,choice)
        winner_check,balance = win_check(player_total,dealer_total,bet,balance,choice)
        if winner_check==True:
            break
        elif winner_check == False and choice == 1:
            choice = player_choice()
        else:
            pass

    if balance == 0:
        print('You are too poor to bet more. Better luck next time.')
        break
    else:
        pass

    play_answer,bet = play_again(balance)
    del winner_check,player_hand,dealer_hand,cards,player_score,dealer_score,player_total,dealer_total,temp
\$\endgroup\$
2
\$\begingroup\$

I would be more mindful of the space that you have around operators. PEP8 recommends single spaces around operators unless you want to help distinguish different operator precedences. I find, for example, self.card_no%13 to read very poorly. It isn't easy to readily see that there's a % operator in there. It looks like it's a part of the name. I also think a line like print('*'*40) would be much clearer with spacing:

print('*' * 40)

In introduction, you're calling int on user input outside of a try. Making sure you're accounting for bad user input is important. You don't want to have the whole thing crash just because the user accidentally typed in 10a instead of 10.


if winner_check==True:
    break
elif winner_check == False and choice == 1:
    choice = player_choice()
else:
    pass

This has a few notable things:

  • == True and == False are unnecessary. Just negate the condition or use the condition directly.

  • In the elif, check_winner must be false. If it was true, the previous condition would have triggered.

  • The else is useless. You do not need an else unless you need some code to run when all other conditions are false. You're just passing though, which is a no-op.

  • Note the inconsistency of your spacing. Within two lines of each other, you have winner_check==True and winner_check == False. Even if you didn't want to follow PEP8, you should at least be consistent.

I'd write this as:

if winner_check:
    break

elif choice == 1:
    choice = player_choice()

At the bottom you have:

 del winner_check,player_hand,dealer_hand,cards,player_score,dealer_score,player_total,dealer_total,temp

I'm not sure why though. You are not required to delete references when you're done with them. That data will be freed when it goes out of scope. You only need to use del if for some reason you really don't want a variable to be in scope later on, within the same scope.


I say this a lot, but don't write a plain except: unless you have a very good reason (like you want to do a catch-all to log errors, and can handle arbitrary catastrophic failure):

try:
    choice = int(input('Do you want to Hit(1) or Stay(2)?'))
except:
    . . .

You're using the try to catch a ValueError from int, so that's what you should be catching:

try:
    choice = int(input('Do you want to Hit(1) or Stay(2)?'))
except ValueError:
    . . .

You don't want to accidentally catch an error that was caused by a programming error.


On the subject of spacing, look at win_check. It's one giant block of text. Not only are you missing spaces around the operators (including ,), you're missing empty lines. I would write this function as:

def win_check(player_total, dealer_total, bet, balance, choice):
    temp = Bank(bet, balance)

    if player_total == 21:
        print('Congrats! You won!')
        balance = temp.transaction_win()
        return True, balance

    elif dealer_total == 21 and choice == 2:
        print('Sorry, you lost your bet')
        balance = temp.transaction_loss()
        return True, balance

    elif player_total > 21:
        print('Bust! Sorry, you lost your bet')
        balance = temp.transaction_loss()
        return True, balance

    elif dealer_total < 21 and dealer_total > player_total and choice == 2:
        print('Sorry, you lost your bet')
        balance = temp.transaction_loss()
        return True, balance

    elif dealer_total > 21 and choice == 2:
        print('Congrats! You won!')
        balance = temp.transaction_win()
        return True, balance

    else:
        return False, balance

Yes, this is much bulkier. It has breathing room though for your eyes to rest at while reading. I found that my eyes kept losing their place while scanning over the function. There weren't any good "landmarks" to reference.

Also note how you repeatedly return True, balance. I'm drawing a blank at the moment, but there's almost definitely a clean way of reducing that redundancy.


I question the need for the Bank class. You only ever use it once inside of win_check. Are you really gaining anything from using them? Why not just subtract from the balance that was passed it? Needlessly wrapping code in a class just muddies its purpose. If all you need to do is add or subtract a number, just use + or -.

It may be worth it if you passed the Bank object around instead of using it solely in the one function. That would only make sense though if the bet never changed.


All imports should be at the very top unless you have a good reason to do otherwise (like you're doing importing in a try because you aren't sure what modules are available.


This isn't at all a real concern, but I'll just mention that in:

if choice not in [1,2]:

The [1, 2] should really be a set. It won't make any real difference here, but it's a good habit to get into. Use the correct structure for the job. Just change it to {1, 2}.


Python 3 has f-strings that make string interpolation easier. Instead of writing:

'Available balance: {}'.format(balance)

Write:

f'Available balance: {balance}'  # Note the f

That reads much nicer.



I don't think I can justify avoiding going downstairs for Thanksgiving any longer 😉.

Good luck and happy (early) Thanksgiving.

\$\endgroup\$
3
  • \$\begingroup\$ Thank you for the feedback. I am aware of the spacing and formatting issue. I do try to make the code more readable but since I am just starting to code again after a couple of years, it's taking some time. As for the else statement in the winner_check block, I initially did not have it there since it is unnecessary. However, the code would not compile and would get stuck. So I thought maybe it expects an else to be there. Could that be the case? FYI I use Jupyter notebooks. Hope you had a great Thanksgiving :) \$\endgroup\$ – Digvijay Rawat Oct 14 '19 at 5:14
  • \$\begingroup\$ @DigvijayRawat A missing else should never cause an error. At worst it would cause a linter warning. I can't see why a warning would happen there though. I'd have to see the error message to be able to say. \$\endgroup\$ – Carcigenicate Oct 14 '19 at 5:17
  • \$\begingroup\$ There would not be a warning or an error, the code would just get stuck on that statement. Sometimes the Jupyter kernels also bug out, I will check again. \$\endgroup\$ – Digvijay Rawat Oct 14 '19 at 5:26

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.