3
\$\begingroup\$

Attached below, is the full program for the simple, text-based blackjack game that I have recently finished developing for Python. Upon review, I was wondering if anyone out there would be able to offer any suggestions for any improvements to my code that could improve functionality or perhaps reduce the length of the program.

#J.Corbett
#Blackjack game prototype - text based
#04/09/17

#import lines
import time
import random
import sys

#set default integer values
PLYRcardValue = 0
DLRcardValue = 0
PLYRcard1 = 0
PLYRcard2 = 0
PLYRcard3 = 0
DLRcard1 = 0
DLRcard2 = 0
DLRcard3 = 0

#set True/False statements
play = True
PLYR_ace1 = False
PLYR_ace2 = False
PLYR_ace3 = False
DLR_ace1 = False
DLR_ace2 = False
DLR_ace3 = False

print("Blackjack Protoype - Version: 101\n\n")

#"open file" function - for reading
def readFile(filename):
  myList = []
  file = open(filename,"r")
  for line in file:
    currentLine = line.strip().split(",")
    myList.append(currentLine[0])
  return myList

#open external file "deck" and assign to array
deck = readFile("deck.txt")

#"check if a card is an Ace" function
def checkAce (card,deck,ace):
    if card == deck[0] or card == deck[13] or card == deck[26] or card == deck[39]:
        ace = True
    else:
        ace = False
    return card, ace

#calling function example: PLYRcard1, PLYR_ace1 = checkAce(PLYRcard1, deck, PLYR_ace1)

#"calculate value of dealt hand" function
def HandValueCalc(card,hand,deck):
  card = deck[random.randint(0,51)] 
  if card == deck[0]:
    hand += 1
  elif card == deck[1]:
    hand += 2
  elif card == deck[2]:
    hand += 3
  elif card == deck[3]:
    hand += 4
  elif card == deck[4]:
    hand += 5
  elif card == deck[5]:
    hand += 6
  elif card == deck[6]:
    hand += 7
  elif card == deck[7]:
    hand += 8
  elif card == deck[8]:
    hand += 9
  elif card == deck[9]:
    hand += 10
  elif card == deck[10]:
    hand += 10
  elif card == deck[11]:
    hand += 10
  elif card == deck[12]:
    hand += 10
  elif card == deck[13]:
    hand += 1
  elif card == deck[14]:
    hand += 2
  elif card == deck[15]:
    hand += 3
  elif card == deck[16]:
    hand += 4
  elif card == deck[17]:
    hand += 5
  elif card == deck[18]:
    hand += 6
  elif card == deck[19]:
    hand += 7
  elif card == deck[20]:
    hand += 8
  elif card == deck[21]:
    hand += 9
  elif card == deck[22]:
    hand += 10
  elif card == deck[23]:
    hand += 10
  elif card == deck[24]:
    hand += 10
  elif card == deck[25]:
    hand += 10
  elif card == deck[26]:
    hand += 1
  elif card == deck[27]:
    hand += 2
  elif card == deck[28]:
    hand += 3
  elif card == deck[29]:
    hand += 4
  elif card == deck[30]:
    hand += 5
  elif card == deck[31]:
    hand += 6
  elif card == deck[32]:
    hand += 7
  elif card == deck[33]:
    hand += 8
  elif card == deck[34]:
    hand += 9
  elif card == deck[35]:
    hand += 10
  elif card == deck[36]:
    hand += 10
  elif card == deck[37]:
    hand += 10
  elif card == deck[38]:
    hand += 10
  elif card == deck[39]:
    hand += 1
  elif card == deck[40]:
    hand += 2
  elif card == deck[41]:
    hand += 3
  elif card == deck[42]:
    hand += 4
  elif card == deck[43]:
    hand += 5
  elif card == deck[44]:
    hand += 6
  elif card == deck[45]:
    hand += 7
  elif card == deck[46]:
    hand += 8
  elif card == deck[47]:
    hand += 9
  elif card == deck[48]:
    hand += 10
  elif card == deck[49]:
    hand += 10
  elif card == deck[50]:
    hand += 10
  else:
    hand += 10
  return card, hand

#calling function example: PLYRcard1, PLYRcardValue = HandValueCalc(PLYRcard1,PLYRcardValue,deck)

#"check if hand is bust" function
def checkBust (handValue):
    if handValue > 21:
        bust = True
        print("Gone bust!")
    else:
        bust = False
    return bust

#calling function example: PLYRbust = checkBust(PLYRcardValue)

#initiate game

if play == True:
#setvalues for the players cards, and check if any are aces
    PLYRcard1, PLYRcardValue = HandValueCalc(PLYRcard1, PLYRcardValue, deck)
    PLYRcard2, PLYRcardValue = HandValueCalc(PLYRcard2, PLYRcardValue, deck)
    PLYRcard1, PLYR_ace1 = checkAce(PLYRcard1, deck, PLYR_ace1)
    PLYRcard2, PLYR_ace2 = checkAce(PLYRcard2, deck, PLYR_ace2)
    time.sleep(1)
    print("You have been dealt these two cards:",PLYRcard1,"and",PLYRcard2)
    time.sleep(1)
    print("The value of your hand is:",PLYRcardValue)
    time.sleep(1)
#let player choose value for Ace, if an ace is present (potential for a function?)
    if PLYR_ace1 == True or PLYR_ace2 == True:
        ace_choice = input("You have been dealt an ace. The ace can be valued one, or eleven. \nWhich value do you want?:")
        if ace_choice == 1:
            PLYRcardValue += 1
        elif ace_choice == 11:
            PLYRcardValue += 11
        else:
            print("Unknown Error: Please restart the program.")
            sys.exit()
        print("The value of your hand is now:",PLYRcardValue)
    choice = input("\nWould you like to stick or split?:")
    if choice == "split":
#set value for the players third card, and check if an ace is present
        PLYRcard3, PLYRcardValue = HandValueCalc(PLYRcard3, PLYRcardValue, deck)
        PLYRcard3, PLYR_ace3 = checkAce(PLYRcard3, deck, PLYR_ace3)
        print("You drew:", PLYRcard3)
        time.sleep(0.5)
#choice for value of ace
        if PLYR_ace3 == True:
            ace_choice = input("You have been dealt an ace. The ace can be valued one, or eleven. \nWhich value do you want?:")
            if ace_choice == 1:
                PLYRcardValue += 1
            elif ace_choice == 11:
                PLYRcardValue += 11
            else:
                print("Unknown Input: Please restart the program.")
                sys.exit()
        print("Your hand is now worth:",PLYRcardValue)  
    time.sleep(1)
#check if the hand is bust
    PLYRbust = checkBust(PLYRcardValue)

#CPU's turn

#set values for dealers hand, check if aces
    DLRcard1, DLRcardValue = HandValueCalc(DLRcard1, DLRcardValue, deck)
    DLRcard2, DLRcardValue = HandValueCalc(DLRcard2, DLRcardValue, deck)
    DLRcard1, DLR_ace1 = checkAce(DLRcard1, deck, DLR_ace1)
    DLRcard2, DLR_ace2 = checkAce(DLRcard2, deck, DLR_ace2)
    time.sleep(1)
#random choice for ace value
    if DLR_ace1 == True or DLR_ace2 == True:
            ace_DLR = random.randint(1,2)
            if ace_DLR == 1:
                DLRcardValue+= 1
                print("The dealer has decided to value the Ace at: 1")
            elif ace_DLR == 2:
                DLRcardValue += 11
                print("The dealer has decided to value the Ace at: 11")
            else:
                print("Unknown Error: Please restart the program.")
                sys.exit()
    print("\nThe Dealer's hand is worth:",DLRcardValue)

#random choice for dealer splitting
    DLRchoice = random.randint(1,3)
    time.sleep(3)
    if DLRchoice == 3:
        print("\nThe dealer has decided to split.")
#set value for dealers third card, check if ace
        DLRcard3, DLRcardValue = HandValueCalc(DLRcard3, DLRcardValue, deck)
        DLRcard3, DLR_ace3 = checkAce(DLRcard3, deck, DLR_ace3)
#ace's value, random choice
        if DLR_ace3 == True:
            ace_DLR = random.randint(1,2)
            if ace_DLR == 1:
                DLRcardValue+= 1
            elif ace_DLR == 2:
                DLRcardValue += 11
            else:
                print("Unknown Error: Please restart the program.")
                sys.exit()

        print("The Dealer's hand is now worth:",DLRcardValue)
    else:
        print("\nThe dealer has decided to stick.")
    time.sleep(1)
#check if dealer's hand is bust
    DLRbust = checkBust(DLRcardValue)
    time.sleep(2)

#Decide winner
    if PLYRbust == True and DLRbust == True and DLRcardValue < PLYRcardValue:
        print("\nThe Dealer is closer to 21! \nDealer wins!")
    elif PLYRbust == True and DLRbust == True and DLRcardValue > PLYRcardValue:
        print("\nThe Player is closer to 21! \nPlayer wins!")
    elif PLYRbust == True and DLRbust == False:
        print("\nThe Dealer is closer to 21! \nDealer wins!")
    elif PLYRbust == False and DLRbust == True:
        print("\nThe Player is closer to 21! \nPlayer wins!")
    elif PLYRcardValue > DLRcardValue and PLYRcardValue < 21:
        print("\nThe Player is closer to 21! \nPlayer wins!")
    elif PLYRcardValue < DLRcardValue and DLRcardValue < 21:
        print("\nThe Dealer is closer to 21! \nDealer wins!")
    elif PLYRcardValue == DLRcardValue:
        print("It's a draw!")
    else:
        print("\nUnknown error, please Restart the program.")
\$\endgroup\$
  • \$\begingroup\$ Since this site is Code Review, "improvements" is implied for all questions. No need to mention it in the title. \$\endgroup\$ – 200_success Sep 7 '17 at 20:10
  • \$\begingroup\$ Follow-up question \$\endgroup\$ – 200_success Nov 6 '17 at 19:40
2
\$\begingroup\$

There's a lot to change in this code. So much so, that I'll just start with a few items.

First, you read in something from "deck.txt" but it's not clear what. I'd suggest that you eliminate this entirely. Just hard-code the values in your program - a deck of cards isn't likely to change very much in the next year or two.

Also, you don't validate the deck you read in. You just assume it has the right size and contents. That's super bogus, since users love to mess with configuration files. If you're going to read in the deck, you'll need to add some validation code.

Next, the card valuation (HandValueCalc): this is nuts. Look up the .index() method and build a parallel list of values. Or do the math: j = index modulo 13, then j if j < 10 else 10.

Finally, I'll suggest that you break up the huge block of code at the bottom of the file into some sensible functions. A main game function, some "turn" functions, etc.

See if you can chop off a bunch of lines, update your question, and we'll try again.

\$\endgroup\$
  • \$\begingroup\$ Thank you for your suggestions! I am currently working on chopping up the large segment at the bottom into some smaller functions, and I also agree that the card validation is insane. Off my own knowledge, I was unsure of any other way to validate and update the values of each cards, and what you see there is all I could come up with. \$\endgroup\$ – James Corbett Sep 6 '17 at 19:17
  • \$\begingroup\$ I do also agree that the deck size could be validated, but for the purpose of the code - a school project - it will be kept in an environment where it will remain untampered, unless done so by myself. Thank you for pointing out the changes that need made to me, though! I'll see what I can do. \$\endgroup\$ – James Corbett Sep 6 '17 at 19:23
  • \$\begingroup\$ Consider building a "card_value" dictionary. \$\endgroup\$ – Austin Hastings Sep 6 '17 at 19:41
4
\$\begingroup\$

I'm only going to review your functions, as your if play == True code is in no way anywhere near how I play blackjack.

  • Python has a style guide - PEP8. It's best if you follow it so that fellow Python programmers can read and manipulate your code easier. It's mostly simple things like rather than calling a function checkAce, you call it check_ace. Make sure you have a space after most commas, so (card,deck,ace) becomes (card, deck, ace). Don't put spaces between a functions name and the arguments. And so rather than using checkAce (card,deck,ace), you use check_ace(card, deck, ace).
  • In check_ace, rather than manually checking if card is equal to something, and then if it's not you check if it's equal to something else. Just check if it's in an object. For example you can use 'a' in ('a', 'b').
  • In read_file, you should use a with statement, it mostly comes down to correctly closing the file. If you wish to know more you can read another one of my answers.
  • In read_file, you may want to use a list comprehension, rather than manually building a list. This is as list comprehensions are simpler to read. For example [i + 1 for i in range(10)] would return all the numbers from 1 to 10 inclusive.
  • I'd recommend that you make your functions have as close to one responsibility each. And so in check_bust, I'd highly recommend that you remove the print. Allowing for a simple single line function.
  • In hand_value_calc, you have about one hundred lines of if a == b[i]: c += mutate(i). There are a couple of problems with this:

    • The variable card is overwritten by deck[index]. This means that your ifs are mostly useless.
    • You know what index is, however you manually iterate all values of it. Instead just calculate it. Take min(9, random.randint(0, 51) % 13) + 1.
    • Finally this doesn't remove a card from the deck, and so multiple users can get the same card.

And so, for all bar the last error I raised, you can reduce your functions to these:

def read_file(filename):
    with open(filename, 'r') as file:
        return [
            line.strip().split(',')[0]
            for line in file
        ]

def check_ace (card, deck, ace):
    return card, card in {deck[0], deck[13], deck[26], deck[39]}

def hand_value_calc(card, hand, deck):
    index = random.randint(0, 51)
    return deck[index], hand + min(9, index % 13) + 1

def check_bust (hand_value):
    return hand_value > 21

I would go through your code more, however, there are some major problems with your code. Instead I'd highly recommend rethinking how you handle the deck and cards. They should probably both be custom classes, which allows for easier to read code. I'd also highly recommend that you allow the user to just play the game, and for your program to care about scoring. Why is the first question you're asked if you draw an ace be:

You have been dealt an ace. The ace can be valued one, or eleven.
Which value do you want?

I, the user, just want to play, and to assign the ace a specific value at a later date. At the beginning I may want the ace to be an 11, however, I later may want it to be a 1.


The way I'd go about doing this, is to give cards names, 'Ace of Spades', and also give them the possibility of multiple values, 1 and 11. This allows the Aces to work correctly. We should also make a 'card holder', with some helper functions such as values. This can then loop through all the cards in the holder, and return all the combinations of values each card can have. This can be constructed by using:

from collections import deque


class Card:
    def __init__(self, name, value):
        self.name = name
        if isinstance(value, int):
            self.value = [value]
        else:
            self.value = value


def _perms(prevs, list):
    if not list:
        yield prevs
        return
    group = list.popleft()
    for item in group:
        yield from _perms(prevs + [item], list)
    list.appendleft(group)


def perms(list):
    yield from _perms([], deque(list))


class CardHolder(list):
    def values(self):
        return perms(card.value for card in self)

After this, you can focus on what the AI does, and also allow users to enter input. This should have a common 'action phase', but different 'pick phases'. For example a very simple 'build' and 'pick phase' could be:

# make deck
deck = CardHolder([
    Card('Ace of Spades', [1, 11]),
    Card('Two of Spades', 2),
    ...
    Card('King of Spades', 10),
    ...
])

# build initial hand
hand = CardHolder([
    deck[0],
    deck[12]
])

# simple AI pick phase
if (any(sum(value) == 21 for value in hand.values()) or 
     all(sum(value) > 15 for value in hand.values())):
    command = 'stand'
else:
    command = 'hit'
\$\endgroup\$
  • \$\begingroup\$ Upon testing the "hand_value_calc" function you designed, I discovered it doesn't work with any of the card games I've previously designed, and a few that I've created today alone. The values within my "deck" array are actually read in from an external file containing the 52 cards as unicode symbols, and the function doesn't update the "card" value for display later, it either leaves it as "0" or as " ", depending on how I define it at the start of the program. \$\endgroup\$ – James Corbett Sep 8 '17 at 13:11
  • \$\begingroup\$ To elaborate: def hand_value_calc(card, hand, deck): return card, hand + min(9, random.randint(0, 51) % 13) + 1 cardTest = hand_value_calc(cardTest,PLYRcardValue,deck) print(cardTest) Will display "(0, 7)", where "0" should be the card, and "7" is the cards value. \$\endgroup\$ – James Corbett Sep 8 '17 at 13:14
  • \$\begingroup\$ @JamesCorbett It works for me. Can you provide some non-working I/O? \$\endgroup\$ – Peilonrayz Sep 8 '17 at 13:19
  • \$\begingroup\$ Of course, give me a a few minutes to do a little more testing and I'll get back to you. \$\endgroup\$ – James Corbett Sep 8 '17 at 13:22
  • \$\begingroup\$ Apologies in advance for the terrible formatting, but I'm sure you can work it out. Code and Inputs: Card_Test = 0 Card_Value = 0 def Hand_Value_Calc(card, hand, deck): return card, hand + min(9, random.randint(0, 51) % 13) + 1 Card_Test, Card_Value = Hand_Value_Calc(Card_Test, Card_Value, deck) print("The card is:", Card_Test, "and it is worth:", Card_Value) Outputs: The card is: 0 and it is worth: 10 Card_Test should be displayed as a symbol, a "🂪" for example and based off the Card_Value, but it isn't updated to said symbol and so it remains as "0". \$\endgroup\$ – James Corbett Sep 8 '17 at 13:33

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.