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
1 Answer 1
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 anelse
unless you need some code to run when all other conditions are false. You're justpass
ing though, which is a no-op.Note the inconsistency of your spacing. Within two lines of each other, you have
winner_check==True
andwinner_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.
-
\$\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– Digvijay Rawat2019εΉ΄10ζ14ζ₯ 05:14:17 +00:00Commented Oct 14, 2019 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– Carcigenicate2019εΉ΄10ζ14ζ₯ 05:17:32 +00:00Commented Oct 14, 2019 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– Digvijay Rawat2019εΉ΄10ζ14ζ₯ 05:26:51 +00:00Commented Oct 14, 2019 at 5:26
Explore related questions
See similar questions with these tags.