I think the core problem here is that your fib
function has two entirely separate responsibilities:
- Handling user input and validation; and
- Calculating Fibonacci numbers.
From the name, I'd say it should only be doing #2!
Instead, structure your program like:
def positive_integer():
"""Loops until the user enters a positive integer."""
...
def fib(n):
"""Prints first n Fibonacci numbers."""
if n < 0:
raise ValueError
prev = 0
curr = 1
for terms in range(n): # note simplification here
nxt = prev + curr
print str(curr),
prev = curr
curr = nxt
fib(positive_integer())
Now you have two functions, each with a clear single responsibility. This makes each one less complex; the range
call is now simpler, for example, as it can assume n
is already an integer (and if not, the user gets a sensible TypeError
to tell them what went wrong).
For writing positive_input
see Asking the user for input until they give a valid response; a loop is better than recursion for this.
As a further refactor, you can actually cut the three lines
nxt = prev + curr
prev = curr
curr = nxt
to just one:
prev, curr = curr, curr + prev
Also note that, in general, you should keep your try
blocks as short as possible and your except
clauses as specific as possible. This makes them more useful when reading the code ("I think this one thing could go wrong, and in that case we can do this about it") and means that legitimate problems aren't quietly ignored. In particular, you should almost never be dealing directly with Exception
(either in raise
or except
), it's far too broad a case to handle usefully.