I tried to write this code as concisely as possible. Is this the best way to do it?
def collatz(n):
"""
Generator for collatz sequence beginning with n
>>> list(collatz(10))
[5, 16, 8, 4, 2, 1]
"""
while n != 1:
n = n / 2 if n % 2 == 0 else 3*n + 1
yield int(n)
3 Answers 3
The only improvement I see here is to divide n
by 2 using //
(since we are dealing with Python 3.x) and to remove the explicit conversion to int (int(n)
):
while n != 1:
n = n // 2 if n % 2 == 0 else 3*n + 1
yield n
Also, I suggest you put a single space before and after the multiplication operator in 3*n
, so that it becomes 3 * n
.
Hope that helps.
I would use while n > 1
, just in case someone tries to call this function with n=0
or even a negative n
.
-
2\$\begingroup\$ Along those lines OP should probably mention in the docstring that
n
is a positive integer. \$\endgroup\$Dair– Dair2016年10月20日 23:36:40 +00:00Commented Oct 20, 2016 at 23:36
I expect collatz(1)
to produce 1
. Rather, it generates an empty sequence. So, in my opinion, the function should immediately yield n
before the loop.
-
\$\begingroup\$ You're right, that was an oversight \$\endgroup\$Vermillion– Vermillion2016年10月21日 03:19:55 +00:00Commented Oct 21, 2016 at 3:19
Explore related questions
See similar questions with these tags.