2
\$\begingroup\$

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!

\$\endgroup\$

2 Answers 2

Reset to default
6
\$\begingroup\$

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

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

  1. draw a card
  2. check if the card is >= 11 points
  3. 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.

\$\endgroup\$
1
  • 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\$ Commented Oct 25, 2020 at 20:12
3
\$\begingroup\$

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}"
\$\endgroup\$
1
  • 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\$ Commented Oct 25, 2020 at 0:19

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.