7
\$\begingroup\$

How can I shorten my code for this? There are 6 variable sets of numbers, someone picks a number between 1-63, if their number is in a set, you add the first number of the list to the magic number variable. If it is not, you don't add anything, and move to the next set. I have used multiple while loops, but want a more effective way to do this.

number_set1 = [2, 3, 6, 7, 10, 11, 14, 15, 18, 19, 22, 23, 26, 27, 30, 31, 34, 35, 38, 39, 42, 43, 46, 47, 50, 51, 54, 55, 58, 59, 62, 63]
number_set2 = [8, 9, 10, 11, 12, 13, 14, 15, 24, 25, 26, 27, 28, 29, 30, 31, 40, 41, 42, 43, 44, 45, 46, 47, 56, 57, 58, 59, 60, 61, 62, 63]
number_set3 = [4, 5, 6, 7, 12, 13, 14, 15, 20, 21, 22, 23, 28, 29, 30, 31, 36, 37, 38, 39, 44, 45, 46, 47, 52, 53, 54, 55, 60, 61, 62, 63]
number_set4 = [16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63]
number_set5 = [1, 3, 5, 7, 9, 11, 13, 15, 17, 19, 21, 23, 25, 27, 29, 31, 33, 35, 37, 39, 41, 43, 45, 47, 49, 51, 53, 55, 57, 59, 61, 63]
number_set6 = [32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63]


answer = ''
magic_number = 0
index = 0

print('I can read your mind... think of a number between 1 to 63 (inclusive)')
print('Don\'t tell me... just be sure to keep it in your mind... I\'ll work it out')
print('by having you answer a few, simple questions....  :)')



answer = input('Would you like to play? [Yes|No] :')
if answer != "Yes" and answer != "No":
    print("\nPlease enter either 'Yes' or 'No'.")

while answer == 'Yes' and index == 0:
    question = input('Is your number in deck 1? [Yes|No] :')
    if question == 'Yes':
        magic_number = magic_number + number_set1[0]
        index = index + 1
    if question == 'No':
        index = index + 1
    
    
while answer == 'Yes' and index == 1:
    question = input('Is your number in deck 2? [Yes|No] :')
    if question == 'Yes':
        magic_number = magic_number + number_set2[0]
        index = index + 1
    if question == 'No':
        index = index + 1

while answer == 'Yes' and index == 1 or index == 2:
    question = input('Is your number in deck 3? [Yes|No] :')
    if question == 'Yes':
        magic_number = magic_number + number_set3[0]
        index = index + 1
    if question == 'No':
        index = index + 1

while answer == 'Yes' and index == 2 or index == 3:
    question = input('Is your number in deck 4? [Yes|No] :')
    if question == 'Yes':
        magic_number = magic_number + number_set4[0]
        index = index + 1
    if question == 'No':
        index = index + 1

while answer == 'Yes' and index == 3 or index == 4:
    question = input('Is your number in deck 5? [Yes|No] :')
    if question == 'Yes':
        magic_number = magic_number + number_set5[0]
        index = index + 1
    if question == 'No':
        index = index + 1

while answer == 'Yes' and index == 4 or index == 5:
    question = input('Is your number in deck 6? [Yes|No]: ')
    if question == 'Yes':
        magic_number = magic_number + number_set6[0]
        index = index + 1
    if question == 'No':
        index = index + 1

    

# Display the secret number to the screen and amaze your friends!  ; )
print('The number you are thinking about is.....', magic_number)
\$\endgroup\$
0
5
\$\begingroup\$

Welcome to Code Review! Congratulations on having a working game/project :)

I have a few pointers to help you minimizing the code length, and probably make it easily extensible. Some of these might be considered personal quirks; so YMMV.

The deck

You're asking the user if their number; at a given state of the run; in any of the given decks. However, for a new player; you never really show the decks ever.

Is the user supposed to remember the entire deck? Perhaps showing the deck before asking if the number is in the same would be better, unless you're using printed cards to act as the deck.

Functions

Use functions for doing something that you see being repeated. In your case, one of the examples is asking user for Yes/No. Let a function ask for user input, and return True/False for y/n values, and the while loop can live inside this little function.

User experience

Asking user to input Yes/No seems counter intuitive in the sense that you want them to input these exactly as shown. In general, a lot of cli programs accept y/Y/Yes/yes and variations thereof to be the same. You can have a validation with a small if-clause for acceptable values like so:

if choice not in {"Yes", "No"}:
    print("Please enter Yes/No")

if-main clause

Put the core logic of your run inside the famous if __name__ == "__main__" block. I will not go into details on the benefits, and just leave a link to this excellent Stack Overflow discussion about the same.

Mathematics

The game works using the concept of boolean algebra. Each set of numbers is actually only setting one of the bits in the binary representation. You can leverage this fact to generate the table dynamically (or to a custom upper limit) and let another function just set the bit value at correct position in the binary representation be set to \$ 1 \$.

\$\endgroup\$
2
\$\begingroup\$

First, let's observe that your code is just a series of the same repeating pattern:

while answer == 'Yes' and index == 0:
    question = input('Is your number in deck 1? [Yes|No] :')
    if question == 'Yes':
        magic_number = magic_number + number_set1[0]
        index = index + 1
    if question == 'No':
        index = index + 1

You check for answer == 'Yes', which is really a check back against the very first question, "would you like to play?" So you can remove that part of the code by simply indenting all your repeated paragraphs under a single if statement:

if answer == 'Yes':
    while index == 0:
        question = input('Is your number in deck 1? [Yes|No] :')
        if question == 'Yes':
            magic_number = magic_number + number_set1[0]
            index = index + 1
        if question == 'No':
            index = index + 1

    ... next while ...

Next, let us observe that your repeated paragraphs really do follow the same pattern. There is a question, a yes-branch, a no-branch, and you add the first member of the number list if the answer is yes.

That's a perfect candidate for a loop, but what should we loop on? Obviously, the only thing that changes: the number_set!

for numset in (number_set1, number_set2, number_set3, ... ):
    while index == 0:   # FIXME: this is wrong!
        ...
            magic_number += numset[0]
        ...

    

Unfortunately, there's that pesky index variable, which wants to increase from 0 by one each time we pass through a number set. You could use the enumerate() built-in function for this, like:

for nsi, numset in enumerate((number_set1, number_set2, ...)):
    while index == nsi:
        ... as before ...

Alternatively, you could eliminate the index variable and use the break keyword when you were ready to stop a loop:

while True:    # Runs forever, since True is always true
    question = ...
    if question in ('Yes', 'No'):
        if question == 'Yes':
            magic_number += number_set1[0]
        break  # stop the while loop
\$\endgroup\$
0
\$\begingroup\$

Consider this an extension to @hjpotter's answer, with some more details. I agree with the basic points. First, congrats on getting something working! And, the main problem I see is the user experience--the game itself looks like it works great. Let's focus on repetition in this review.

Functions and retry logic

Add a function to get the answer. I'm changing the variable name from question to answer, since, that's what's in the variable--Yes and No are answers. You named it this because you already had an answer variable in use--in the future use answer2 or something like that if you really need a new name.

def ask_question(question):
    answer = input(question)
    while answer not in ["Yes", "No"]:
        answer = input('Please input [Yes|No] only: ')
    return answer

As hjpotter suggests, this would be a great place to print the deck for the user, who probably won't know the answer otherwise.

By separating out the retry logic into the function, we've simplified what each deck loop looks like a lot:

answer = ask_question('Is your number in deck 1? [Yes|No] :')
if answer == 'Yes':
    magic_number = magic_number + number_set1[0]
index += 1

In fact, we no longer care about index at all--it was only being used for retry logic, so we can drop that line.

Looping

Next, we'll move th the rest of the program. Currently the main logic looks like this:

answer = ask_question('Is your number in deck 1? [Yes|No] :')
if answer == 'Yes':
    magic_number = magic_number + number_set1[0]

answer = ask_question('Is your number in deck 2? [Yes|No] :')
if answer == 'Yes':
    magic_number = magic_number + number_set2[0]

answer = ask_question('Is your number in deck 3? [Yes|No] :')
if answer == 'Yes':
    magic_number = magic_number + number_set3[0]

answer = ask_question('Is your number in deck 4? [Yes|No] :')
if answer == 'Yes':
    magic_number = magic_number + number_set4[0]

answer = ask_question('Is your number in deck 5? [Yes|No] :')
if answer == 'Yes':
    magic_number = magic_number + number_set5[0]

answer = ask_question('Is your number in deck 6? [Yes|No] :')
if answer == 'Yes':
    magic_number = magic_number + number_set6[0]

That's a lot better already. But let's simplify even more using a loop:

number_sets = {1: number_set1, 2: number_set2, 3: number_set3, 4: number_set4, 5: number_set5, 6: number_set6}

for deck_number, number_set in number_sets.items():
    answer = ask_question('Is your number in deck {}? [Yes|No] :'.format(deck_number))
    if answer == 'Yes':
        magic_number = magic_number + number_set[0]

Putting it together

The final program might look something like this

import sys

number_set1 = [2, 3, 6, 7, 10, 11, 14, 15, 18, 19, 22, 23, 26, 27, 30, 31, 34, 35, 38, 39, 42, 43, 46, 47, 50, 51, 54, 55, 58, 59, 62, 63]
number_set2 = [8, 9, 10, 11, 12, 13, 14, 15, 24, 25, 26, 27, 28, 29, 30, 31, 40, 41, 42, 43, 44, 45, 46, 47, 56, 57, 58, 59, 60, 61, 62, 63]
number_set3 = [4, 5, 6, 7, 12, 13, 14, 15, 20, 21, 22, 23, 28, 29, 30, 31, 36, 37, 38, 39, 44, 45, 46, 47, 52, 53, 54, 55, 60, 61, 62, 63]
number_set4 = [16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63]
number_set5 = [1, 3, 5, 7, 9, 11, 13, 15, 17, 19, 21, 23, 25, 27, 29, 31, 33, 35, 37, 39, 41, 43, 45, 47, 49, 51, 53, 55, 57, 59, 61, 63]
number_set6 = [32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63]
number_sets = {1: number_set1, 2: number_set2, 3: number_set3, 4: number_set4, 5: number_set5, 6: number_set6}

def ask_question(question):
    answer = input(question)
    while answer not in ["Yes", "No"]:
        answer = input('Please input [Yes|No] only: ')
    return answer

if __name__ == '__main__':
    answer = input('Would you like to play? [Yes|No] :')
    if answer == "No":
        sys.exit(0)

    print('I can read your mind... think of a number between 1 to 63 (inclusive)')
    print('Don\'t tell me... just be sure to keep it in your mind... I\'ll work it out')
    print('by having you answer a few, simple questions....  :)')

    magic_number = 0
    for deck_number, number_set in number_sets.items():
        answer = ask_question('Is your number in deck {}? [Yes|No] :'.format(deck_number))
        if answer == 'Yes':
            magic_number = magic_number + number_set[0]

    # Display the secret number to the screen and amaze your friends!  ; )
    print('The number you are thinking about is.....', magic_number)
\$\endgroup\$
2
  • 2
    \$\begingroup\$ the ask_question should just return True/False imo, so that you don't have to later check against valid string values (suppose all variations of y/Y/yes/YES...) \$\endgroup\$
    – hjpotter92
    Oct 19 at 6:09
  • \$\begingroup\$ I agree. I thought it would make the rewrite harder to read for the original author--on balance though this was a mistake. \$\endgroup\$ Oct 22 at 22:20

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.