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\$ Commented 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\$ Commented 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\$ Commented Jun 20, 2019 at 15:35

1 Answer 1

Reset to default
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\$ Commented 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\$ Commented 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\$ Commented May 7, 2022 at 0:01

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.