6
\$\begingroup\$

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
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Jul 4, 2014 at 18:33
\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

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:

  1. Note adherence to the Python Style Guide, PEP-0008, e.g. using lowercase_with_underscores for variable names;
  2. random.choice is used rather than hard-coding bounds for range (if you stick with choosing the index, at least assign the index first rather than picking a card based on a random number then using index to get that same number back);
  3. The card is now just an integer, making comparison easy without needing to find the index. The "face" (e.g. 'Jack') is only looked up when displaying to the user (using dict.get to return either the face string if it's in the dictionary or the card value if not);
  4. 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:
    1. Picking the next card;
    2. Comparing the cards; and
    3. Telling the user they've won or lost;
  5. A function forces the user to provide valid input. This will loop until they pick one of 'h', 'l' or 's' (case-insensitive);
  6. while True replaces the odd behaviour with x that never gets incremented; and
  7. 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 calling game.

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.

answered Jul 4, 2014 at 21:21
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.