1
\$\begingroup\$

I would like to improve my code. I am not a Python expert, so I am glad for any help. I would like to remove redundancies, unnecessary code, and change things that might shorten it.

#!/usr/bin/env python3
import skilstak.colors as c
import random as r
def computerPlay():
    while cscore < 17:
        chand.append(deck[position])
        del deck[position]
        cscore = sum(chand)
    else:
        if 11 in chand:
            chand.remove(11)
            chand.append(1)
            cscore = sum(chand)
            if cscore > 21:
                winOrLose()
            else:
                computerPlay()
def winOrLose():
    global cwins
    global wins
    global credits 
    cBet = int(input("How many credits would you like to bet? > "))
    cscore = sum(chand)
    score = sum(hand)
    if cscore > 21:
        cbust = "y"
        if score > 21:
            bust = "y"
        else:
            bust = "n"
    else:
        cbust = "n"
        if score > 21:
            bust = "y"
        else:
            bust = "n"
    print(c.cl + "The computer's hand was" + str(chand))
    print("Your hand was" + str(hand))

    if cbust == "y":
        if bust == "y":
            print(c.yellow + "It's a tie!" + c.base01)
            print("P",wins,": C",cwins)
        else:
            print(c.green + "You won!" + c.b01)
            wins += 1
            print("P",wins,": C",cwins)
            credits += cBet
            print("You now have",credits)
    elif bust == "y":
        if cbust == "y":
            print(c.yellow + "It's a tie!" + c.b01)
            print("P",wins,": C",cwins)
        else:
            print(c.red + "You lost."+ c.b01)
            cwins += 1
            print("P",wins,": C",cwins)
            credits -= cBet
            print("You now have",credits)
    else:
        if cscore > score:
            print(c.red + "You lost."+ c.b01)
            cwins += 1
            print("P",wins,": C",cwins)
            credits -= cBet
            print("You now have",credits)
        elif cscore < score:
            print(c.green + "You won!" + c.b01)
            wins += 1
            print("P",wins,": C",cwins)
            credits += cBet
            print("You now have",credits)
        else:
            print(c.yellow + "It's a tie!" + c.b01)
            print("P",wins,": C",cwins)
    playAgain()
def playAgain():
    again = input("Would you like to play again?(y/n) > ")
    if again == "y":
        print(c.cl)
        del hand[:]
        del chand[:]
        deck = [2,3,4,5,6,7,8,9,10,10,10,10,11] * 4
        position = 0 #Which card to pull out of deck
        shuffleAndStart()
        newcard()
    else:
        exit()

def newcard():
    score = sum(hand)
    while score <= 21:
        userCard = input(c.b01 + "Would you like a new card?(y/n) > ")
        if userCard == "y":
            hand.append(deck[position])
            score = sum(hand)
            del deck[position]
            print(c.cl + "Your hand contains " + str(hand) + " for a total of",score,"points.")

        else:
            computerPlay() 
            break
    else:
        if 11 in hand:
            hand.remove(11)
            print(c.cl + "Your ace valued 11 was changed into a 1")
            hand.append(1)
            score = sum(hand)
            print("You now have"+str(hand)+"and",score,"Points")
            if score > 21:
                print("Oh no! You busted with:",score,"points and a hand of" + str(hand))
                discard.append(hand)
                computerPlay()
        else:
            newcard()
    else:
        print("You busted with:",score,"points and a hand of" + str(hand))
        discard.append(hand)
        computerPlay()
def shuffleAndStart():
    print(c.cl)
    r.shuffle(deck)
    r.shuffle(deck)#shuffles the deck twice
    hand.append(deck[position]) #adds new card to hand
    score = sum(hand) # gets score
    del deck[position] #removes card from deck
    print("Your hand contains a:",score)
if __name__ == '__main__':
    print(c.cl + c.b01 + "Welcome to BlackJack v.1.0! Created by Peter")
    print("You have 500 credits.")
    deck = [2,3,4,5,6,7,8,9,10,10,10,10,11] * 4
    hand = []
    credits = 500
    chand = []
    cwins = 0
    wins = 0
    discard = []
    position = 0 #Which card to pull out of deck
    shuffleAndStart()
    newcard()
\$\endgroup\$
2
  • \$\begingroup\$ I would like to remove redundancies, unnecessary code, or something to shorten it. \$\endgroup\$ – retep Jan 16 '16 at 14:46
  • \$\begingroup\$ In that case look into the single responsibilities principle which essentially means that a function should only be doing one thing. I would create a main loop function and split your functions into chunks of code doing their own thing. \$\endgroup\$ – user94917 Jan 16 '16 at 14:51
2
\$\begingroup\$

General guidelines for good Python code

Starting with PEP-8. It lays guidelines for naming and layout. Top-level things you should be focused on when writing Python. So things like using snake_case for naming instead of camelCase(), avoiding global variables, not using single-letter names for non-trivial objects.

Don't alias your imports like this:

import random as r

You only use this module in one place, and r.shuffle(deck) is a lot more cryptic than random.shuffle(deck).

The prolific use of globals makes it hard to figure out what depends on what. Take the first function:

def computer_play():

It takes no arguments, and returns nothing. But it does a LOT. And the signature should make that clear:

def computer_play(computer_hand, deck):

One advantage of not having global variables is that it makes it easy to test. With computer_play taking its input as arguments, you can just write docstring tests. And that'll also clarify what it is that this function does.

Dealing from the deck

So you start with your deck:

deck = [2,3,4,5,6,7,8,9,10,10,10,10,11] * 4

And then you shuffle() it. There is no reason to shuffle it twice, by the way. But then the way you "deal" from the deck is repeatedly taking the first element and deleting it. This is inefficient - erasing from the front of a list is \$O(n)\$.

We don't need to erase anything at all though. Just iterate through the deck! First, let's separate the "deck creation" from the "hand dealing":

def create_new_deck():
    deck = [2,3,4,5,6,7,8,9,10,10,10,10,11] * 4
    random.shuffle(deck)
    return iter(deck)

We return an iterator, so that taking the next element is as single as just next():

def deal_hand(deck):
    return [next(deck), next(deck)]

You started the player off with just the one card, but you start with two in BlackJack. Also, you want to give the computer a hand too, so that you know how to play:

deck = create_new_deck()
player_hand = deal_hand()
cpu_hand = deal_hand()

# show player his hand and 2nd cpu card
# then prompt, etc.

With the deck being an iterator, now drawing a hand changes from the cryptic:

 hand.append(deck[position])
 del deck[position]

To:

hand.append(next(deck))

Dealer play

You have a while...else loop, but there's no break. Which means the else will always be run. So that's unnecessary.

The recursion is unnecessarily confusing too. The structure of the loop should be:

def computer_play(cpu_hand, deck):
    while computer_should_hit(cpu_hand):
        cpu_hand.append(next(deck))
    return cpu_hand

That's it. Just implement computer_should_hit() in the right way. Deal with the busting logic elsewhere. Don't remove the aces and replaces them with 1s, just handle that in the function itself. This way, the function reads like the rules of the game, and makes it easier to understand.

Booleans

Python has True/False for boolean logic. Don't use "y"/"n".

\$\endgroup\$
3
  • \$\begingroup\$ Thanks so much! Could you combine it all and give it back to me? \$\endgroup\$ – retep Jan 16 '16 at 15:57
  • \$\begingroup\$ Thanks so much! Could you explain computer_should_hit to me? \$\endgroup\$ – retep Jan 16 '16 at 16:08
  • \$\begingroup\$ @retep It should return True is the computer should take another card, False otherwise. \$\endgroup\$ – Barry Jan 16 '16 at 19:58

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.