11
\$\begingroup\$

This code creates a problem for the user to solve (very simple problem) it will then check if the user has got it right and will say so.

import random
import time
def mathsquestion():

    attempts=2
    bananas1 = random.randrange(1,40)
    bananas2 = random.randrange(30,70)
    print('lucy has', bananas1,' bananas and tracy has', bananas2,' if both of them put their bananas together and split them evenly')
    question = float(input('how many bananas do each of them get?'))


    if question == (bananas1+bananas2)/2:
        print ('correct')
        restart=input('want to try again')
        if restart == ('yes'):
            mathsquestion()

    else:
        print('wrong')
        attempts-=1
        print('you have',attempts,'attempts remaining')
        question2 = float(input('how many bananas do each of them get?'))
        if question2 == (bananas1+bananas2)/2:
            print('yay finally')
            restart=input('want to try again')
            if restart == ('yes'):
                mathsquestion()

        else:
            print('wrong')
        attempts-=1
        print('you have',attempts,'attempts remaining')
        print('shutting down')

    if attempts == 0:
        time.sleep(3)
        quit()

mathsquestion()
\$\endgroup\$
18
\$\begingroup\$

General Feedback

The good thing is that it is quite clear what your code does. You have for the most part clear variable names and not too much clutter. However I would change bananas1 to lucys_bananas and bananas2 to tracys_bananas.

It seems you are familiar with another programming language. Python has a style guide PEP 8 which explains in excruciating detail how to structure your code. I whole heartily recommend skimming through it and follow it.

You write attempts = 3 however according to PEP 8 it is commended to use CAPITALIZED_WITH_UNDERSCORES for constants, UpperCamelCase for class names, and lowercase_separated_by_underscores for other names.

Error handling Logic

The first thing that came to mind was

question = float(input('how many bananas do each of them get?'))

I guess you thought that it was a good idea to make question into a float since (bananas1+bananas2)/2 creates a float. The idea is good but what happens if I guess something that is not a float? Your whole program crashes! A good way to avoid this is to catch the error before it happens

question = input('how many bananas do each of them get?')
try:
    question = float(question)
except:
    print('Error, number of bananas must be an integer')
    continue 

With how your code is currently setup this does not quite work. However we can make a few more changes. Getting half number of bananas, and having to work with floats when bananas is integers is in my opinion a tad strange. However it is possible to make sure that the number of bananas always is evenly divisible. If Lucy has a odd number, then Tracy has to have a odd number of bananas. Similarly if Lucy has an even number, then Tracy has to have an even number of bananas. Since \$(2L+1)+(2T+1)=2(T+L+1)\$ and \$2L+2T=2(T+L)\$. This can be coded as follows.

    lucy_bananas = get_bananas(1, 40)
    even_lucy_bananas = lucy_bananas % 2
    tracy_bananas = get_bananas(30+even_lucy_bananas, 70+even_lucy_bananas, 2)

To switch behaviour we can put the creation of the random variables into a new function. Se the revised code below

The next part is strange and not very pythonic.

if attempts == 0:
    time.sleep(3)
    quit()

What is the purpose of time.sleep(3)? Also quit() is something which you should never have to use. The standard is return however a better solution is to wrap the entire function in a while loop. That way it can terminate once attempts reaches zero.

Further tips and changes

What your function does is clear however it would be much clearer if you split it into smaller parts. A good practice is that each function has a single purpose, as it stands your main function does too many things.

  1. Unnecessary import the time is not neccecary
  2. You do not need to import the entire random libary, you are only using a single function.
  3. You should use the if __name__ == "__main__": module in your answer.
  4. Duplication of code you write question2 = float(input('how many bananas do each of them get?')) more than once.
  5. The above is not really a question is it? A better name would be answer
  6. Error handling, you convert answer to a float, what if this causes an error? You have no way to detect this.
  7. mathsquestion() should not call itself, you should leave this to a main function.
  8. The creation on random variables should be put into a seperate function
  9. Clear variable names are good, but you should also have included a short explenation of what your code does. This is done by using docstrings
  10. The printing could be improved by using pythons awesome formating options

I have improved the first few points in the code below. I will leave the rest to you.

from random import randrange

MAX_ATTEMPTS = 3
LUCY_MIN_BANANA = 1
LUCY_MAX_BANANA = 40
STACY_MIN_BANANA = 30
STACY_MAX_BANANA = 70
EVEN_BANANAS = True

def get_bananas():
    lucy_bananas = randrange(LUCY_MIN_BANANA, LUCY_MAX_BANANA)
    if EVEN_BANANAS:
        offset = lucy_bananas % 2
        step = 2
    else:
        offset = 0
        step = 1      
    tracy_bananas = randrange(STACY_MIN_BANANA+offset, STACY_MAX_BANANA-offset, step)
    return lucy_bananas, tracy_bananas


def mathsquestion():

    lucy_bananas, tracy_bananas = get_bananas()
    bananas_evenly_divided = float(lucy_bananas+tracy_bananas)/2

    print('lucy has', lucy_bananas, ' bananas and tracy has', tracy_bananas,
          ' if both of them put their bananas together and split them evenly')
    print('how many bananas do each of them get?')

    attempts = MAX_ATTEMPTS
    while attempts > 0:

        answer = input('answer: ')
        try:
            answer = float(answer)
        except:
            print('Error, the number of bananas must be an integer')
            continue

        if answer == bananas_evenly_divided:
            return True
        print('attempts remaining: ', attempts-1)
        attempts -= 1
    return False


def main():
    want_to_play = True
    while want_to_play:
        guess_correct = mathsquestion()
        if guess_correct == True:
            print('Congratulations!)')
        else:
            print('Better luck next time!')
        restart = input('want to try again? [y/n]: ').lower()
        if restart not in ['yes', 'y']:
            want_to_play = False


if __name__ == '__main__':

    main()
| improve this answer | |
\$\endgroup\$
  • 3
    \$\begingroup\$ Please, do not suggest using bare except. Always specify the exception you're willing to handle. Here you want except ValueError. \$\endgroup\$ – 301_Moved_Permanently Jun 27 '16 at 5:43
  • \$\begingroup\$ thankyou for the suggestions will use them and rewrite my code. I am very new to programming and have just used my general knowledge of what I know to create this. \$\endgroup\$ – J.Fitz Jun 27 '16 at 6:16

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.