4
\$\begingroup\$

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)
asked Oct 20, 2016 at 17:38
\$\endgroup\$

3 Answers 3

4
\$\begingroup\$

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.

answered Oct 20, 2016 at 17:58
\$\endgroup\$
4
\$\begingroup\$

I would use while n > 1, just in case someone tries to call this function with n=0 or even a negative n.

answered Oct 20, 2016 at 21:52
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Along those lines OP should probably mention in the docstring that n is a positive integer. \$\endgroup\$ Commented Oct 20, 2016 at 23:36
4
\$\begingroup\$

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.

answered Oct 21, 2016 at 2:57
\$\endgroup\$
1
  • \$\begingroup\$ You're right, that was an oversight \$\endgroup\$ Commented Oct 21, 2016 at 3:19

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.