Recently I have been learning Python programming, for about a week. I have covered all basic stuff - loops, strings, input masks - and have challenged myself by making a little game. It isn't much, that I know, but I was just wandering whether or not I have done everything correctly.
Any feedback regarding functions and methods used would be greatly appreciated.
import random
cards = ["2","3","4","5","6","7","8","9","10","Jack","Queen","King","Ace"]
suits = ["Spades","Hearts","Clubs","Diamonds"]
x = 1
while x < 20:
for i in range(1,9):
cardVal = cards[random.randint(0,12)]
cardNum = cards.index(cardVal)
suitVal = suits[random.randint(0,3)]
if i == 1:
print("Your starting card is the",cardVal,"of",suitVal)
HoL = str(input("Higher (H), Lower (L) or Same (S): "))
if HoL == "H":
newCval = cards[random.randint(0,12)]
newCnum = cards.index(newCval)
if newCnum < cardNum + 1:
print("Sorry, you fail. Card is",newCval,"of",suitVal)
break
elif newCnum > cardNum:
print("Good job! Card is",newCval,"of",suitVal)
else:
print("Sorry, you fail. Card is",newCval,"of",suitVal)
break
if HoL == "L":
newCval = cards[random.randint(0,12)]
newCnum = cards.index(newCval)
if newCnum > cardNum - 1:
print("Sorry, you fail. Card is",newCval,"of",suitVal)
break
elif newCnum < cardNum:
print("Good job! Card is",newCval,"of",suitVal)
else:
print("Sorry, you fail. Card is",newCval,"of",suitVal)
break
if HoL == "S":
newCval = cards[random.randint(0,12)]
newCnum = cards.index(newCval)
if newCnum != cardNum:
print("Sorry, you fail. Card is",newCval,"of",suitVal)
break
else:
print("Good job! Card is",newCval,"of",suitVal)
print("Congratulations!")
repeat = str(input("Enter 'Yes' to run again: "))
if repeat != "Yes":
break
1 Answer 1
Here is an alternative implementation, with explanatory comments below. This introduces quite a few additional Python concepts:
from operator import ge, le, ne
import random
def get_valid_input(choices=set("hls")):
while True:
ui = input("Higher (H), Lower (L) or the Same (S)? ").lower()
if ui in choices:
return ui
print("Not a valid choice, please try again.")
def game(rounds=8):
cards = range(2, 15)
faces = {11: 'Jack', 12: 'Queen', 13: 'King', 14: 'Ace'}
suits = ["Spades", "Hearts", "Clubs", "Diamonds"]
comparisons = {'h': le, 'l': ge, 's': ne}
pass_template = "Good job! The card is the {0} of {1}."
fail_template = "Sorry, you fail. The card is the {0} of {1}."
while True:
card = random.choice(cards)
suit = random.choice(suits)
print("The first card is the {0} of {1}.".format(faces.get(card,
card),
suit))
for round_ in range(rounds):
next_card = random.choice(cards)
next_suit = random.choice(suits)
if comparisons[get_valid_input()](next_card, card):
print(fail_template.format(faces.get(next_card,
next_card),
next_suit))
break
print(pass_template.format(faces.get(next_card,
next_card),
next_suit))
card, suit = next_card, next_suit
else:
print("Congratulations!")
repeat = input("Play again? ").lower()
if repeat not in "yes":
print("Goodbye.")
break
if __name__ == "__main__":
game()
Comments:
- Note adherence to the Python Style Guide, PEP-0008, e.g. using
lowercase_with_underscores
for variable names; random.choice
is used rather than hard-coding bounds forrange
(if you stick with choosing the index, at least assign the index first rather than picking a card based on a random number then usingindex
to get that same number back);- The
card
is now just an integer, making comparison easy without needing to find theindex
. The "face" (e.g.'Jack'
) is only looked up when displaying to the user (usingdict.get
to return either the face string if it's in the dictionary or the card value if not); - A dictionary, with the
operator
module, is also used to reduce duplication in the main loop. The trick is recognising the duplication - in all cases, you are comparing two cards with a particular rule. The following now only happen in one place, reducing chance of error:- Picking the next card;
- Comparing the cards; and
- Telling the user they've won or lost;
- A function forces the user to provide valid input. This will loop until they pick one of
'h'
,'l'
or's'
(case-insensitive); while True
replaces the odd behaviour withx
that never gets incremented; and- The whole thing is wrapped in a function, which makes it easier to
import
elsewhere later and has also allowed the "magic number"8
to be variable when callinggame
.
One additional thing you might want to consider is actually simulating a deck of cards. As it stands, the same card could appear twice, i.e. the game is currently being played as if the card were being put back into the deck each time. This may or may not be what you're aiming for - I would expect to play as if the (up to) eight cards were being laid out on the table one after another.
Explore related questions
See similar questions with these tags.