2
\$\begingroup\$

I wrote the following code for the Collatz exercise: The Collatz Sequence

Any feedback is appreciated.

Write a function named collatz() that has one parameter named number. If number is even, then collatz() should print number // 2 and return this value. If number is odd, then collatz() should print and return 3 * number + 1.

def collatz(n):
    while n > 1:
        if n % 2 == 0:
            n = int(n // 2)
            print (n)
        elif n % 2 == 1:
            n = int(3 * n + 1)
            print (n)

n = int(input("Enter a number: "))
collatz (n)
\$\endgroup\$
3
  • 4
    \$\begingroup\$ @drapozo It would be welcome if you'd provide either a link or a small description of the problem. \$\endgroup\$
    – dfhwze
    Jun 6, 2019 at 6:39
  • 2
    \$\begingroup\$ It´s a problem form the Book automate the boring stuff with python. The problem is "The Collatz Sequence" and can be found in this link. automatetheboringstuff.com/chapter3 \$\endgroup\$
    – drapozo
    Jun 7, 2019 at 14:27
  • 3
    \$\begingroup\$ When adding additional information you should edit your question instead of adding a comment. I have added that information to your post. Learn more about comments including when to comment and when not to in the Help Center page about Comments. \$\endgroup\$ Jun 20, 2019 at 15:35

1 Answer 1

4
\$\begingroup\$

A few simple observations:

  •     if n % 2 == 0:
            ...
        elif n % 2 == 1:
            ...
    

    Here, we know that if n % 2 isn't 0, it must be 1, since n is an integer. So that elif can be simply else, making it simpler to read. More experienced programmers will reverse the test, knowing that 1 is the only truthy result of n % 2, so write if n % 2: ... ; else ....

  •     if ...:
            ...
            print (n)
        else:
            ...
            print (n)
    

    The print that's present in both branches could be placed after the if/else, since it doesn't depend on the condition.

  • The results of the arithmetic expressions are already int, so no conversion is necessary.

  • The if/else could be reduced to a single line:

    n = 3 * n + 1 if n % 2 else n // 2
    

    but to me, that looks less clear, so I don't recommend that level of terseness here.


Modified code

Applying the observations above, I get a simpler version of the function:

def collatz(n):
    while n > 1:
        if n % 2:
            n = 3 * n + 1
        else:
            n = n // 2
        print(n)
\$\endgroup\$
3
  • \$\begingroup\$ Thanks for your observations. In the simpler version you wrote i think you meant if n % 2 == 1. \$\endgroup\$
    – drapozo
    Jun 7, 2019 at 14:24
  • \$\begingroup\$ No, I didn't mean if n % 2 == 1. Because n % 2 can only be 0 or 1, it's simpler to just test whether n % 2 is truthy. \$\endgroup\$ Jun 10, 2019 at 8:34
  • \$\begingroup\$ I think these are all good suggestions. For what it's worth though, I find the original if statements easier to read. \$\endgroup\$ May 7, 2022 at 0:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.