HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

My own twist on the classic "Guessing Game" script in Python

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scripttheclassicowngamepythonguessingtwist

Problem

This is a (letter, or whatever the user decides to change it to) guessing game I wrote which implements both a mechanic to randomly change the order of elements in a list, as well as limit the number of attempts you get to complete the game. There's not much else to say about it. I'm just looking for some suggestions on where I can condense things and better optimize my code.

import random

samples = random.sample(['x', 'y', 'z'], 3)
keys = []

for i in samples:
    str(keys.append(i))

attempts_left = 3

print("X, Y, and Z are arranged in random order.")
print("The object of the game is to guess them in order before you run out of tries.\n")

while len(keys) > 0:

    for i, j in list(enumerate(keys, start=1)):
        key = j

        while attempts_left > 0:

            print("Round {}".format(i))
            guess = input("> ").lower()

            if guess == key:
                i += 1
                print("Correct\n")
                keys.remove(key)
                break

            elif guess not in ['x', 'y', 'z']:
                print("That's not one of the options.")

            else:
                print("Wrong\n")
                attempts_left -= 1

    if attempts_left > 0:
        print("You win!")
        break

    else:
        print("You LOSE!")
        print("Good DAY, sir!")
        break

Solution

Using the right functions

What you want to do is present the string xyz in some order. For that, there's random.shuffle(). Doing sampling is for taking a small number of elements from a large set (think, drawing a poker hand from a deck), and is not well suited for this particular problem. So we can start with:

answer = 'xyz'
random.shuffle(answer)


Note that a string in python is also an iterable, you don't have to make it a list of strings. Also your str() in your loop does nothing - and the loop itself is just a very verbose copy. Just the shuffle suffices.

Your outer loop

Your logic looks like:

while cond:
    ...
    if cond2:
        break
    else:
        break


That is, the outer loop never matters. len(keys) > 0 for sure the first time through and then we never check it again, so we can simply drop it.

Iteration

There's no reason to write list(enumerate(iterable)) if you're just looping over it. That unnecessarily has to bring the entire enumeration into memory to construct the full list. You only need one element at a time - so let the iterable give you one element at a time.

We can also make this much more readable by naming things better:

for round, letter in enumerate(answer):
   ...


Why are we enumerating anyway?

Enumerating is for enumerating. But here, the round number doesn't actually correspond to the letter number. It should just be an external variable:

round = 0
for letter in answer:
    while attempts > 0:
        round += 1

        print 'Round {}'.format(round)
        guess = input('> ').lower()

        if guess == letter:
            print('Correct!')
            break
        elif len(guess) > 1 or guess not in answer:
            print('Invalid option')
        else:
            print('Wrong!')
            attempts -= 1

Code Snippets

answer = 'xyz'
random.shuffle(answer)
while cond:
    ...
    if cond2:
        break
    else:
        break
for round, letter in enumerate(answer):
   ...
round = 0
for letter in answer:
    while attempts > 0:
        round += 1

        print 'Round {}'.format(round)
        guess = input('> ').lower()

        if guess == letter:
            print('Correct!')
            break
        elif len(guess) > 1 or guess not in answer:
            print('Invalid option')
        else:
            print('Wrong!')
            attempts -= 1

Context

StackExchange Code Review Q#110308, answer score: 2

Revisions (0)

No revisions yet.