This is one of my first programmes in python and I really could use some feedback on how to improve. It's meant to be a multiplayer game by handing the computer to the next player at the end of each turn.

import random
import time
import os
import operator

def invalid():
    print('\nINVALID INPUT')
    time.sleep(0.8)
#checks what the players deck adds up to
def deck_check(deck):
    #No idea why but it only works if the card dictionary is also here and not if given to the function as a variable
    card={"1":1, "2":2, "2":2, "3":3, "4":4, "5":5, "6":6, "7":7, "8":8, "9":9, "Jack":10, "Queen":10, "King":10, "Ace":1}
    total=0
    ace_number=0
    if "Ace" in deck:
        #two Loops to make sure all aces are at the end of player_deck
        for a in deck:
            if "Ace" == a:
                deck.remove(a)
                ace_number=ace_number+1
        for _ in range(ace_number):
            deck.append("Ace")
        for t in deck:
            if t=="Ace":
                if total <=10:
                    if ace_number==1:
                        card["Ace"]=11
                    else:
                        card["Ace"]=1
                else:
                    card["Ace"]=1
                ace_number=ace_number-1
            total=total+card[t]
    else:
        for b in deck:
            total=total+card[b]
    return total

while True:
    print('\n1:New Game\n2:Exit Game')
    x = input("Enter(1,2):")
    if x=="1":
        while True:
            player_list=[]
            player_capital={}
            try:
                players = int(input("Enter number of players:"))
            except ValueError:
                invalid()
                continue
            else:
                if players<2:
                    print('\nMinimum 2 players')
                    time.sleep(0.8)
                    continue
            while True:
                try:
                    rounds = int(input("Enter number of rounds:"))
                except ValueError:
                    invalid()
                    continue
                else:
                    if rounds<1:
                        print('\nMinimum 1 round')
                        time.sleep(0.8)
                        continue
                    break
            while True:
                try:
                    money = int(input("Enter how much money all players should start with:"))
                except ValueError:
                    invalid()
                    continue
                else:
                    if money<10:
                        print('\nMinimum 10')
                        time.sleep(0.8)
                        continue
                    break
            for u in range(players):
                v = input("Enter player "+str(u+1)+" name:")
                player_list.append(v)
                player_capital[v]=money
            while True:
                try:
                    bet = int(input("Enter how much all players bet at the beginning of each round:"))
                except ValueError:
                    invalid()
                    continue
                else:
                    if bet<1:
                        print('\nMinimum 1')
                        time.sleep(0.8)
                        continue
                    elif bet>money:
                        print('\nNot enough starting money')
                        time.sleep(0.8)
                        continue
                    break
                        #Loop for rounds
            for r in range(rounds):
                round_result={}
                capital=len(player_list)*bet
                for z in range(len(player_list)):
                    player_capital[player_list[z]] = player_capital[player_list[z]]-bet
                #Rests Deck
                Cards={"1":1, "2":2, "3":3, "4":4, "5":5, "6":6, "7":7, "8":8, "9":9, "Jack":10, "Queen":10, "King":10, "Ace":1}
                Deck={"Hearts":None, "Spades":None, "Clubs":None, "Diamonds":None}
                for n in Deck:
                    Deck[n] = Cards
                #Loop for players
                for p in range(len(player_list)):
                    if player_capital[player_list[p]] <=0:
                        print(f'\n{player_list[p]} is broke')
                        continue
                    player_deck=[]
                    print(f'\nRound: {r+1}\nPlayer {player_list[p]} your turn!\nMake sure no other player can see the screen!')
                    time.sleep(4)
                    for _ in range(2):
                        h = random.choice(list(Deck))
                        g = random.choice(list(Deck[h]))
                        player_deck.append(g)
                        c = Deck[h]
                        del c[g]
                    if set(player_deck)=="Ace":
                        print(f'\nMoney in the game: {capital}\nYour money: {player_capital[player_list[p]]}\nYour Cards: {player_deck}\nBLACKJACK!')
                        break
                    else:
                        while True:
                            total=deck_check(player_deck)
                            if total>21:
                                print('\nYour bust!')
                                time.sleep(1)
                                break
                                continue
                            else:
                                print(f'\nMoney in the game: {capital}\nYour money: {player_capital[player_list[p]]}\nYour Cards: {player_deck} Adding up to: {total}\n1:Draw additional card\n2:Increse bet\n3:End turn')
                                e = input("Enter(1,2,3):")
                                if e=="1":
                                    h = random.choice(list(Deck))
                                    g = random.choice(list(Deck[h]))
                                    player_deck.append(g)
                                    c = Deck[h]
                                    del c[g]
                                elif e=="2":
                                    try:
                                        print("")
                                        q = int(input("Enter by how much do you want to incresen bet:"))
                                    except ValueError:
                                        invalid()
                                        continue
                                    if q > player_capital[player_list[p]]:
                                        print('\nYou dont have that much money!')
                                        continue
                                    else:
                                        capital=capital+q
                                        player_capital[player_list[p]] = player_capital[player_list[p]]-q
                                elif e=="3":
                                    round_result[player_list[p]]=total
                                    break
                                else:
                                    invalid()
                    print('\nNext Player')
                    print ("\n" * 100)
                try:
                    round_winner = max(round_result.items(), key=operator.itemgetter(1))[0]
                except ValueError:
                    print('\nNext Round')
                    time.sleep(1)
                    continue
                player_capital[round_winner]=player_capital[round_winner]+capital
                print(f'\nRound Winner is {round_winner}\nNext Round')
                time.sleep(1)
            winner = max(player_capital.items(), key=operator.itemgetter(1))[0]
            print(f'\n{winner} is the winner!\nGAME OVER!')
            break
    elif x=="2":
        break
    else:
        invalid()
  • 3
    A good review (imo) should address the need to split this into functions – D. Ben Knoble Nov 18 at 20:24
up vote 0 down vote accepted

Will finish review later if I have a chance! The most important thing is to create functions for repeated code. Using classes instead of list/tuple/dictionary-based structures also helps. May I refer you to The Zen of Python?

Tips in the order came up with them:

  1. You have an unused import os.
  2. IMO, it makes more sense to use the term "hand" to refer to each player's cards and "deck" to refer to the cards not yet dealt.
  3. check_deck would be better named sum_hand.
  4. Extracting your input code into a function called input_integer would greatly improve readability.
  5. IMO, it doesn't really make sense to add pauses.
  6. It might make sense to extract your prompts as constants to allow for decreased clutter and easier internationalization.
  7. Code such as for i in range(len(player_list)): can should be refactored into for player in player_list: (or for i, player in enumerate(player_list): if the index is necessary). This is both more efficient and more readable.
  8. If you use f-strings, there's no point in using concatenation.
  9. Make sure to use variable names that are as descriptive as possible.
  10. Make sure to use a main function and an if __name__ == '__main__': check. See this answer (as well as the other answers to that question) for more information.
  11. Use in-place operators (e.g. a = a + b is (almost) identical in functionality to a += b).
  12. I may have forgotten to mention some things.

Remember to use functions to your advantage and use descriptive names!

https://repl.it/@solly_ucko/BlackJack-CodeReview

Instead of manually enter every item in the command

card={"1":1, "2":2, "2":2, "3":3, "4":4, "5":5, "6":6, "7":7, "8":8, "9":9, "Jack":10, "Queen":10, "King":10, "Ace":1}

you may use

card = {str(i): i for i in range(1, 10)}
card.update(dict(zip("Jack Queen King Ace".split(), 3 * [10] + [1])))

and, similarly, instead of

Deck={"Hearts":None, "Spades":None, "Clubs":None, "Diamonds":None}

you may use

Deck = dict(zip("Hearts Spades Clubs Diamonds".split(), 4 * [None]))

or, even better (thanks to Aaron Hall) -

Deck = dict.fromkeys("Hearts Spades Clubs Diamonds".split())

and instead of commands like

ace_number=ace_number+1

you may use

ace_number += 1

And, it would be nice to follow the PEP 8 - Style Guide for Python Code suggestions.

  • 3
    Isn't that second line simpler and clearer as: card.update({*dict(zip(["Jack", "Queen", "King"], [10] * 3)), "Ace": 1}) or even (with more repetition) card.update({"Jack": 10, "Queen": 10, "King": 10, "Ace": 1}) – Graham Nov 18 at 18:19
  • @Graham, yes, may be, it probably depends on the personal preferences. – MarianD Nov 18 at 18:29
  • 1
    Deck = dict.fromkeys("Hearts Spades Clubs Diamonds".split()) – Aaron Hall Nov 18 at 23:13
  • @AaronHall - nice - I'm going to put it in my answer. – MarianD Nov 19 at 12:52

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.