1
\$\begingroup\$

I just started coding programs with python. As a challenge I wanted to program something to print out the Fibonacci Numbers. It looks like I've done what I wanted to do (I checked the first few numbers) and because I just started coding, I obviously don't know if this is a legitimate code and somebody would code it like that. Keep in mind that I have only done some basic lessons in python so I just want to get some feedback and advice for better code.

def spiral(sequence):
 numbers = []
 for i in sequence:
 numbers.append(0)
 if i == 0 or i == 1:
 numbers[i] = 1
 continue
 numbers[i] = numbers[i-1] + numbers[i-2]
 return numbers
times = range(int(input("How many times do you want to go through? ")))
fibonacci = spiral(times)
print(fibonacci)
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Jun 29, 2018 at 20:37
\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

The way sequence is used guarantees that the results will be wrong for the majority of inputs. It's clearly intended that sequence should be a continuous range starting at 0. So rather than require the range as input, it would make more sense to require the limit as input.


 numbers.append(0)
 if i == 0 or i == 1:
 numbers[i] = 1
 continue
 numbers[i] = numbers[i-1] + numbers[i-2]

This confused me briefly until I figured out that it appends 0 as a placeholder and then overwrites it with the appropriate Fibonacci number. Following the principle of KISS (keep it simple), it would make more sense to append the Fibonacci number directly.

IMO the expression is short enough to use a ternary:

 numbers.append(numbers[i-1] + numbers[i-2] if i > 1 else 1)

Those two changes give

def spiral(n):
 numbers = []
 for i in range(n):
 numbers.append(numbers[i-1] + numbers[i-2] if i > 1 else 1)
 return numbers
times = int(input("How many times do you want to go through? "))
fibonacci = spiral(times)
print(fibonacci)

The names seem quite opaque. Why spiral? Why times? They both seem to assume some context which isn't documented in the code.


It would be more Pythonic to write a generator, but since you say you're a beginner this may be a concept which you haven't encountered yet.

answered Jun 29, 2018 at 21:33
\$\endgroup\$
1
  • \$\begingroup\$ Thank you for the feedback. The names of the variables came from translating the variable names from my language to english, which wasn't very smart to make them in my language I know, but I had do think of names quick. Gonna work on the naming next time. :) \$\endgroup\$ Commented Jun 30, 2018 at 12:44

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.