Recently I had a python test and, unfortunately, I failed. I'm going to re-do my test and the teacher gave me the tip to work more efficiently and clean. To practice this I made a blackjack game about 2 weeks ago with python and sent it to him to check. he has yet responded and my test is next week. Can anyone take a look and maybe point things out that need improvement? please, I really want to pass this test.
import itertools
import random as rd
from time import sleep as s
#making 3 decks with playing cards and assign them 2 to 14
cards1 = list(itertools.product(range(2, 15),['spade', 'heart', 'diamond', 'club']))
cards2 = list(itertools.product(range(2, 15),['spade', 'heart', 'diamond', 'club']))
cards3 = list(itertools.product(range(2, 15),['spade', 'heart', 'diamond', 'club']))
#combine the 3 decks to make 1
cards = list(cards1+cards2+cards3)
#shuffle deck
rd.shuffle(cards)
def blackjack(cards):
money = 10
while True:
print('you have', money, 'money')
bet = int(input('select amount to bet: \n'))
if money < bet:
print('you dont have that much money....')
else:
playing = True
#draw first card and remove it from the deck
fcard = rd.choice(cards)
cards.remove(fcard)
first_point, first_name = fcard
#check if first card is 11 points or more (to change back to 10 points unless it's ace)
if first_point == 11:
first_point = 10
first_name = str('Jack'+' of '+first_name)
elif first_point == 12:
first_point = 10
first_name = str('Queen'+' of '+first_name)
elif first_point == 13:
first_point = 10
first_name = str('King'+' of '+first_name)
elif first_point == 14:
first_point = 11
first_name = str('Ace'+' of '+first_name)
#show the first drawn card
print(first_point, first_name)
s(0.7)
#draw second card and remove it from the deck
scard = rd.choice(cards)
cards.remove(scard)
second_point, second_name = scard
#checking second card for the same
if second_point == 11:
second_point = 10
second_name = str('Jack'+' of '+second_name)
elif second_point == 12:
second_point = 10
second_name = str('Queen'+' of '+second_name)
elif second_point == 13:
second_point = 10
second_name = str('King'+' of '+second_name)
elif second_point == 14:
second_point = 11
second_name = str('Ace'+' of '+second_name)
#show second card
print(second_point, second_name)
s(0.7)
points = first_point + second_point
#check if first 2 cards make a blackjack
if points == 21:
print('Blackjack!')
bet *= 2
print('you won', bet, 'money')
money += bet
playing = False
print(points, 'points out of 21')
if money == 0:
print('you are broke!')
exit()
#after the first 2 cards i need to determine if the player wants more cards
while playing:
card = input('press enter to draw a card or type x to stop')
if card != 'x':
a = rd.choice(cards)
x, y = a
#going through the same checking system as the first 2 cards
if x == 11:
y = str('Jack'+' of '+second_name)
x = 10
elif x == 12:
y = str('Queen'+' of '+second_name)
x = 10
elif x == 13:
y = str('King'+' of '+second_name)
x = 10
elif x == 14:
y = str('Ace'+' of '+second_name)
x = 11
print(x, y)
s(0.7)
cards.remove(a)
points += x
if points > 21:
print('BUST')
points = 0
playing = False
#if the player has x as input the player stops drawing
elif card == 'x':
playing = False
print(points, 'points')
#let the dealer do the same card drawing
result = dealer_draw(cards)
print('you scored: ', points, '\n', 'the bank scored: ', result)
s(0.7)
#compare obtained points with the dealer's points
if points > result:
print('you win!')
money += bet
elif points == result:
print('draw')
elif points < result:
print('you lose')
money -= bet
elif points == 0 and result == 0:
print('you lose')
money -= bet
def dealer_draw(cards):
#2 empty prints to maintain clear overview
print()
print()
a = 0
#first 2 cards (same as for the player until.....)
cd1 = rd.choice(cards)
cards.remove(cd1)
points_first, name_first = cd1
if points_first == 11:
name_first = str('Jack'+' of '+name_first)
points_first = 10
elif points_first == 12:
name_first = str('Queen'+' of '+name_first)
points_first = 10
elif points_first == 13:
name_first = str('King'+' of '+name_first)
points_first = 10
elif points_first == 14:
name_first = str('Jack'+' of '+name_first)
points_first = 11
print(points_first, name_first)
s(0.7)
cd2 = rd.choice(cards)
cards.remove(cd2)
points_second, name_second = cd2
if points_second == 11:
name_second = str('Jack'+' of '+name_second)
points_second = 10
elif points_second == 12:
name_second = str('Queen'+' of '+name_second)
points_second = 10
elif points_second == 13:
name_second = str('King'+' of '+name_second)
points_second = 10
elif points_second == 14:
name_second = str('Ace'+' of '+name_second)
points_second = 11
print(points_second, name_second)
s(0.7)
#..... here (scroll up)
full_points = points_first + points_second
a += full_points
#have the minimal bank draw set at 16
while a < 16:
print("bank's total = ", a)
s(0.7)
draw = rd.choice(cards)
cards.remove(draw)
add_number, full_name = draw
if add_number == 11:
full_name = str('Jack'+' of '+full_name)
add_number = 10
elif add_number == 12:
full_name = str('Queen'+' of '+full_name)
add_number = 10
elif add_number == 13:
full_name = str('King'+' of '+full_name)
add_number = 10
elif add_number == 14:
full_name = str('Ace'+' of '+full_name)
add_number = 11
print(add_number, full_name)
s(0.7)
a += add_number
print("bank's total = ", a)
s(0.7)
#check if bank scored more than 21 and if so, return 0
if a > 21:
return 0
else:
return a
blackjack(cards)
Any comments are welcome but please keep in in mind that this is my first programming language and I still have a lot to learn. Thanks!
2 Answers 2
I'm sorry but my knowledge in card games is rusty. Please correct me if something is wrong!
Always catch invalid input
Assume that the user is going to enter something, that is prompted to him from this line of code
print('you have', money, 'money')
bet = int(input('select amount to bet: \n'))
select amount to bet:
Now, what if the user accidentally entered E. In this case, your program will fail since it expects input in form of an integer. This is why you should always catch invalid input using Try and Except in Python
try:
bet = int(input("select amount to be: "))
except Exception:
print("Invalid input! Please enter a number\n")
This way, if the user entered
select amount to be: I like python
It would give the user
Invalid input! Please enter a number
Don't exit after invalid / bad input
In your program, if the user enters a bet which is more than the money he has, the program just stops. It won't play again, why should this happen?
You should ask the user to enter valid input again, so that any mistake by him won't result in the immediate termination of the program
while True:
try:
bet = int(input("select amount to be: "))
except Exception:
print("Invalid input! Please enter a number\n")
continue
if bet > money:
print("Bet placed higher than balance!")
continue
break
The best thing to do now is to move this to a separate function called take_input()
, so your blackjack()
function can remain clean, and now taking input is easy
bet = take_input()
Yes, you have written a few more lines of code. But now you know your program will do the right thing when Exceptions occur.
Simplify code - 1
first_name = str('Jack'+' of '+first_name)
Is the same as
first_name = "Jack of " + first_name
You don't need to convert to str
as first_name
is already a string.
The same applies to the following lines that I have extracted from your code.
first_name = str('Queen'+' of '+first_name)
first_name = str('King'+' of '+first_name)
first_name = str('Ace'+' of '+first_name)
Avoid Magic Numbers
Take this example
if first_point == 11:
first_point = 10
first_name = str('Jack'+' of '+first_name)
elif first_point == 12:
first_point = 10
first_name = str('Queen'+' of '+first_name)
elif first_point == 13:
first_point = 10
first_name = str('King'+' of '+first_name)
elif first_point == 14:
first_point = 11
first_name = str('Ace'+' of '+first_name)
10
, 11
, 12
... are known as magic numbers. I had to think while to understand what they were doing here until I finally understood that they were cards.
The good way to deal with this is to use Python's enums.
from enum import Enum
class Card(Enum):
jack = 11
queen = 12
king = 13
....
Correct the values if they are wrong.
Now your if-else thread looks so much clearer to the reader
if first_point == Card.jack.value:
...
elif first_point == Card.queen.value:
...
elif first_point = Card.king.value:
....
Another plus point is that what if you want to change the value of the king from x
to y
. Would you go to the hundreds of places to find where you might have used a numeric constant in context to the king?
here you can just set king.value
to whatever you want.
(削除) import sleep as s
(削除ここまで)
import sleep as s
(削除ここまで)s(0.5)
This also had me confused in the beginning, I had to found out what s
meant. s
isn't meaningful at all, it just confuses anyone who reads your code. However, sleep
clearly implies that you want to... Sleep! Always use meaningful names
Split work into functions
Currently, your blackjack()
function is cluttered with a lot of tasks that should be moved to their own functions. Just like we moved the input procedure into a separate take_input()
function, you can create a lot of meaningful functions like draw_new_card()
that can return a new card from the deck.
Is your code DRY or WET
Excuse my knowledge of card games
You have the procedure
- draw a card
- check if the card is
>=
11 points - print the point and name
Then why repeat the same thing again for the second card? You have written the same thing twice. Once for the first card, and next for the second. you have repeated yourself. The best way is to factor repetition out into a function. So that all you need to do is
def new_card():
card = draw_new_card()
point, name = card
process_card(point, name)
return point, name
# in the blackjack function #
first_point, first_name = new_card()
print(first_point, first_name)
sleep(0.5)
second_point, second_name = new_card()
print(second_point, second_name)
......
You can see that using functions has helped a lot.
-
1\$\begingroup\$ thanks a bunch! i'm going to take a look at your comments 1 by 1 and see if I can improve! \$\endgroup\$liteversion– liteversion2020年10月25日 20:12:38 +00:00Commented Oct 25, 2020 at 20:12
Naming conventions
Just to reinforce the point made by @Aryan Parekh: Don't use meaningless abbreviations eg:
import random as rd
from time import sleep as s
There is no benefit, you've made the code harder to read and understand.
So: use random.choice(cards)
instead of: rd.choice(cards)
. random.choice is self-explanatory.
Good code should be intuitive, that begins with proper naming conventions. Even if you are lazy you should use longer and more descriptive names, your IDE should have autocomplete anyway.
You have variables like a, cd2, x, y that remind me of spaghetti Basic from the 80s. I totally suck at card games so I can't comment much on the algo but I can comment on the code.
Fortunately, you put some comments.
Consistency
You use the choice function a couple times but with very different variables names:
a = rd.choice(cards)
x, y = a
and later:
draw = rd.choice(cards)
cards.remove(draw)
add_number, full_name = draw
I think more consistency is called for here. If you reuse some statements you might as well use the same variable names elsewhere or at least stick to some naming patterns that make sense. draw is a name that makes sense. But add_number really looks like a function name, so I would call it card_number or something like that (even though you are effectively using that variable for incrementing another value).
Repetition
There is repetition in your code eg:
#making 3 decks with playing cards and assign them 2 to 14
cards1 = list(itertools.product(range(2, 15),['spade', 'heart', 'diamond', 'club']))
cards2 = list(itertools.product(range(2, 15),['spade', 'heart', 'diamond', 'club']))
cards3 = list(itertools.product(range(2, 15),['spade', 'heart', 'diamond', 'club']))
First off, some statements are redundant:
#combine the 3 decks to make 1
cards = list(cards1+cards2+cards3)
Since you are concatenating three lists the resulting object is a list object too.
So, cards = cards1 + cards2 + cards3
is sufficient and yields the same result.
cards1/2/3 are exactly the same, so you are repeating the exact same thing 3 times in a row. This is obviously wrong and can be simplified. You could simply write:
cards2 = cards1
cards3 = cards1
even though that is not elegant but at least you avoid repetition and your range is declared just once.
A better way:
cards = list(itertools.product(range(2, 15), ['spade', 'heart', 'diamond', 'club'])) *3
Thus, you've repeated your sequence three times and created a new list.
Since you are using itertools, you could also use itertools.repeat, which gives you a generator, whereas * n
gives you a list, which is just fine here.
Concatenation
draw = rd.choice(cards)
cards.remove(draw)
add_number, full_name = draw
if add_number == 11:
full_name = str('Jack'+' of '+full_name)
add_number = 10
full_name is a string, so you can safely concatenate all these items. Or better yet, use an F-string (Python >= 3.6):
full_name = f"Jack of {full_name}"
-
2\$\begingroup\$ In your Repetition section, you should clarify that your way of creating
cards
creates only 52 distinct card objects in memory with each reference to a card object repeated three times, whereas the original creates a list of 52*3 distinct card objects in memory; see this interactive example. It doesn't affect anything in this case because the cards are modeled as immutable tuples, but if they were mutable and being changed as part of the program flow, this would lead to surprising results. \$\endgroup\$Setris– Setris2020年10月25日 00:19:36 +00:00Commented Oct 25, 2020 at 0:19