Here's the code I'm having trouble shortening. I am currently a beginner in Python 3.x.
from os import system
def fibonacci(terms):
count = 0
x = 0
y = 1
z = x + y
if terms <= 0:
print("Enter a positive integer.")
elif terms == 1:
print("Fibonacci sequence in " + str(terms) + " terms.")
print(x)
else:
print("Printing " + str(terms) + " terms.")
while count < terms:
print(x)
z = x + y
x = y
y = z
count += 1
while True:
user_input = int(input("How many terms? "))
fibonacci(user_input)
loop = input("Again? (Y/n) ")
if loop == "Y" or loop == "":
system("cls")
else:
exit()
I would appreciate if there will be someone who will be able to make this as efficient as possible.
-
2\$\begingroup\$ Ahoy! Welcome to code review. I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to edit and give it a different title if there is something more appropriate \$\endgroup\$Sᴀᴍ Onᴇᴌᴀ– Sᴀᴍ Onᴇᴌᴀ ♦2020年11月23日 04:48:21 +00:00Commented Nov 23, 2020 at 4:48
2 Answers 2
Separate responsibilities
Your functions should, where possible, limit themselves to doing one thing only. This is the single-responsibility principle. In case of fibonacci()
, I expect this function to just calculate the desired Fibonacci number, and not deal with user input and output. This shortens the function and makes it easier to reason about. So:
def fibonacci(terms):
count = 0
x = 0
y = 1
z = x + y
while count < terms:
yield x
z = x + y
x = y
y = z
count += 1
Instead of printing the values, it yield
s them. To print the values, use the following:
terms = int(input("How many terms? "))
for number in fibonacci(terms):
print(number)
You can even go futher and make fibonacci()
an infinite generator, and leave it up to the caller to decide when to stop:
import itertools
def fibonacci():
x = 0
y = 1
while True:
yield x
z = x + y
x = y
y = z
terms = int(input("How many terms? "))
for number in itertools.islice(fibonacci(), terms):
print(number)
Here I used itertools.islice()
to take only the first terms
items from the generator.
If you want to handle invalid input gracefully, I suggest you write another function whose responsibility is asking the user for a postivive integer:
def get_positive_integer(prompt):
while True:
user_input = int(input(prompt))
if ...:
return user_input
print("Enter a positive integer.")
Avoid using os.system()
Using os.system()
to clear the screen is very inefficient and not portable. Of course it is prettier to clear the screen, but if it is not really important for your program, I would just not do it; portability is more important.
-
1\$\begingroup\$ as an addition OP constructs:
print("Fibonacci sequence in " + str(terms) + " terms."
could useformat
instead. \$\endgroup\$Jean-François Fabre– Jean-François Fabre2020年11月22日 16:45:19 +00:00Commented Nov 22, 2020 at 16:45 -
5\$\begingroup\$ @Jean-FrançoisFabre Format is old, f-strings are good \$\endgroup\$user228914– user2289142020年11月22日 16:45:51 +00:00Commented Nov 22, 2020 at 16:45
-
1\$\begingroup\$ yeah, unless you're stuck with a legacy version of python. \$\endgroup\$Jean-François Fabre– Jean-François Fabre2020年11月22日 16:46:17 +00:00Commented Nov 22, 2020 at 16:46
-
\$\begingroup\$ One drawback of f-strings is that they don't really work well in combination with internationalization. \$\endgroup\$G. Sliepen– G. Sliepen2020年11月22日 16:48:09 +00:00Commented Nov 22, 2020 at 16:48
z = x + y
x = y
y = z
is not idiomatic Python. Consider
x, y = y, x + y
Regarding optimization, since you want all terms, there is not much to optimize. If wanted a specific term, a matrix exponentiation would be a way to go.
Explore related questions
See similar questions with these tags.