Asking for Fibonacci numbers, up till 50

Question

I am learning Python, and this is one of the program assignments I have to do.

I need to write code to prompt user to enter Fibonacci numbers continuously until it's greater than 50. If everything correct, screen show 'Well done', otherwise 'Try again'.

While my code is working correctly, I would like to know how I should write it better.

My code:

# Init variables
t = 0
prev_2 = 1
prev_1 = 0
run = 1

# Run while loop to prompt user enter Fibonacci number
while (run):
   text = input('Enter the next Fibonacci number >')
   if (text.isdigit()):
      t = int(text)
      if t == prev_2 + prev_1:
         if t <= 50:
            prev_2 = prev_1
            prev_1 = t
         else:
            print('Well done')
            run = 0
      else:
         print('Try again')
         run = 0
   else:
      print('Try again')
      run = 0

Answer 1

First, run should be a Boolean value (True or False). 1 and 0 work, but they're far less clear. Note how much more sense this makes:

. . .
run = True

while (run):
   text = input('Enter the next Fibonacci number >')
   if (text.isdigit()):
      t = int(text)
      if t == prev_2 + prev_1:
         if t <= 50:
            prev_2 = prev_1
            prev_1 = t
         else:
            print('Well done')
            run = False
      else:
         print('Try again')
         run = False
   else:
      print('Try again')
      run = False

---

Instead of using a run flag though to exit the loop, I think it would be a lot cleaner to just break when you want to leave:

while (True):
   text = input('Enter the next Fibonacci number >')
   if (text.isdigit()):
      t = int(text)
      if t == prev_2 + prev_1:
         if t <= 50:
            prev_2 = prev_1
            prev_1 = t
         else:
            print('Well done')
            break
      else:
         print('Try again')
         break
   else:
      print('Try again')
      break

---

Instead of doing a isdigit check, you can just catch the ValueError that int raises:

while (True):
    text = input('Enter the next Fibonacci number >')
    try:
        t = int(text)
    except ValueError:
        print('Try again')
        break

    if t == prev_2 + prev_1:
        if t <= 50:
            prev_2 = prev_1
            prev_1 = t
        else:
            print('Well done')
            break
    else:
        print('Try again')
        break

This goes with Python's "It's better to ask for forgiveness than permission" philosophy.

---

Some other things:

• Don't put parenthesis around the condition of while and if statements unless you really feel that they help readability in a particular case.

• You don't need to have t assigned outside of the loop. I'd also give t a better name like user_input:

  prev_2 = 1
  prev_1 = 0
  
  # Run while loop to prompt user enter Fibonacci number
  while True:
      text = input('Enter the next Fibonacci number >')
      try:
          user_input = int(text)
      except ValueError:
          print('Try again')
          break
  
      if user_input == prev_2 + prev_1:
          if user_input <= 50:
              prev_2 = prev_1
              prev_1 = user_input
          else:
              print('Well done')
              break
      else:
          print('Try again')
          break
  

Answer 2

A few minor tips on making the code more compact:

1. Remove unneeded variables/initializations. In particular your t variable doesn't need to be initialized outside the loop since its value is always set within the loop before it's used.

2. The run variable can also be eliminated; make the loop while True and use a break to escape it when you're done.

3. Instead of doing text.isdigit() just do the int conversion inside a try block.

4. Combine your two "Try again" cases into a single block of code rather than repeating yourself.

With those changes the code looks like:

# Init previous two values of Fibonacci sequence
prev_2 = 1
prev_1 = 0

# Run while loop to prompt user enter Fibonacci number
while True:
    try:
        t = int(input('Enter the next Fibonacci number >'))
        if t != prev_2 + prev_1:
            raise ValueError('not the next Fibonacci number!')
        if t > 50:
            print('Well done')
            break
        # move to next value in the sequence
        prev_2, prev_1 = prev_1, t
    except ValueError:
        print('Try again')
        break

A larger tip on improving the code would be to separate the two distinct tasks the code is doing: computing the Fibonacci sequence up to 50, and quizzing the user on it. Here's how I might write that:

# Init Fibonacci sequence up to first value past 50 (shfifty-five)
fib = [0, 1]
while fib[-1] <= 50:
    fib.append(fib[-2] + fib[-1])

# Prompt the user for all those values (after the initial 0, 1)
for n in fib[2:]:
    try:
        if int(input('Enter the next Fibonacci number >')) != n:
            raise ValueError('not the next Fibonacci number!')
    except ValueError:
        print('Try again')
        break
else:
    print('Well done')

Now the code is in two smaller parts that are each a lot simpler than the previous version -- the first few lines compute the Fibonacci sequence in a very straightforward way, and the rest of the code simply iterates over the sequence and checks the input against it.

In a larger project you'd probably take these two "chunks" of code and define them as functions (you'd have one function that generates the sequence and another that handles the quizzing), but even within a function it's valuable to think about how you can break a problem does into distinct simple parts (and once you've done that, the next step of refactoring it into smaller functions becomes easy).

Answer 3

The 3 answers already given are great. I'd just like to point out a minor bug with the original code not mentioned in these 3 answers.

The str.isdigit() function can yield false positives. If the user enters 1²₃, the isdigit() method will return True because all the characters are Unicode digits, but int(...) will raise an exception because the characters are not all decimal digits.

The function you wanted was str.isdecimal().

>>> text = "1²₃"
>>> text.isdigit()
True
>>> int(text)
Traceback (most recent call last):
  module __main__ line 141
    traceback.print_exc()
  module <module> line 1
    int(text)
ValueError: invalid literal for int() with base 10: '1²₃'
>>> text.isdecimal()
False
>>> 

Answer 4

Welcome to CodeReview! This is a great first question.

Breaking out of a loop

You don't necessarily need to have a condition at the declaration of the loop, like while (run):. Instead,

# Run while loop to prompt user enter Fibonacci number
while True:
   text = input('Enter the next Fibonacci number >')
   if text.isdigit():
      t = int(text)
      if t == prev_2 + prev_1:
         if t <= 50:
            prev_2 = prev_1
            prev_1 = t
         else:
            print('Well done')
            break
      else:
         print('Try again')
         break
   else:
      print('Try again')
      break

Two other answers just popped up so I will leave this here.

Answer 5

Others have given many great suggestions on the code style, but I'd like to point out that your code's behavior does not exactly match your description of what it should do. You said:

I need to write code to prompt user to enter Fibonacci numbers continuously until it's greater than 50. If everything correct, screen show 'Well done', otherwise 'Try again'.

Based on this description, it sounds like the code should be prompting for numbers continuously, even if they give you a wrong answer, until they give you a number greater than 50. Only then would you reveal whether they had gotten it correct or not.

I didn't see anyone else point this out, and the hardest part of programming is understanding exactly what your program should do. I may be understanding the description wrong, of course, but I would double-check that your program meets the problem specification.