Python - The Collatz Sequence

Question

Can I get a review of my code for the the Collatz Sequence from Chapter three of Automate the Boring Stuff with Python?

The Collatz Sequence

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.

Then write a program that lets the user type in an integer and that keeps calling collatz() on that number until the function returns the value 1. (Amazingly enough, this sequence actually works for any integer—sooner or later, using this sequence, you’ll arrive at 1! Even mathematicians aren’t sure why. Your program is exploring what’s called the Collatz sequence, sometimes called “the simplest impossible math problem.”)

Remember to convert the return value from input() to an integer with the int() function; otherwise, it will be a string value.

Hint: An integer number is even if number % 2 == 0, and it’s odd if number % 2 == 1.

The output of this program could look something like this:

Enter number:
3
10
5
16
8
4
2
1

Input Validation

Add try and except statements to the previous project to detect whether the user types in a noninteger string. Normally, the int() function will raise a ValueError error if it is passed a noninteger string, as in int('puppy'). In the except clause, print a message to the user saying they must enter an integer.

I am mainly wondering if there is a cleaner way to write my solution.

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

def getNum():
    global num
    num = (input("> "))
    try:
        num = int(num)
    except ValueError:
        print('plese enter a number')
        getNum()
getNum()
collatz(num)

Answer 1

First, note how you're duplicating calculations:

print(num//2)
num = num //2

This may not cause issues with this specific code, but it isn't a good practice. You're doing twice as much work as you need to, which can cause performance issues once you start writing more complicated code. Do the calculation once, and save the result. In this case though, all you need to do is reverse those lines and use num:

num = num // 2
print(num)

Also, make sure you have proper spacing around operators, and be consistent.

---

Your if and elif cases are exclusive of each other, and your else should never happen. If the first condition is true, then other must be false and vice-versa. There's no need for the second check. Once rewritten, you'll see that printing in every case isn't necessary. You can just print after:

while num > 1:
    if num % 2 == 0:
        num = num // 2

    else:
        num = 3 * num + 1

    print(num)

Since you're just reassinging num one of two options based on a condition, a conditional expression can be used here cleanly as well:

while num > 1:
    num = (num // 2) if num % 2 == 0 else (3 * num + 1)
    print(num)

The braces aren't necessary, but I think they're useful here due to the number of operators involved.

---

---

Printing the numbers isn't ideal here. In most code, you need to be able to use the data that you produce. If you wanted to analyze the produced sequence, you would have to do something intercept the stdout, which is expensive and overly complicated. Make it a function that accumulates and returns a list. In the following examples, I also added some type hints to make it clearer what the type of the data is:

from typing import List

def collatz(starting_num: int) -> List[int]:
    nums = [starting_num]

    num = starting_num
    while num > 1:
        num = (num // 2) if num % 2 == 0 else (3 * num + 1)
        nums.append(num)

    return nums

Or, a much cleaner approach is to make it a generator that yields the numbers:

# Calling this function returns a generator that produces ints
# Ignore the two Nones, as they aren't needed for this kind of generator
def collatz_gen(starting_num: int) -> Generator[int, None, None]:
    yield starting_num

    num = starting_num
    while num > 1:
        num = (num // 2) if num % 2 == 0 else (3 * num + 1)
        yield num

>>> list(collatz_gen(5))
[5, 16, 8, 4, 2, 1]

---

---

There's a few notable things about getNum:

Python uses "snake_case", not "camelCase".

---

Your use of global num here is unnecessary and confusing. Just like before, explicitly return any data that the function produces:

def get_num() -> int:
    raw_num = input("> ")

    try:
        return int(raw_num)

    except ValueError:
        print('Please enter a number')
        return get_num()

Note how instead of reassigning a global num, we're just returning the number. I also spaced things out a bit, and used some more appropriate names. Conceptually, I'd say that num = input("> ") is wrong. At the time that that runs, num does not contain a number (it contains a string).

---

This isn't a good use of recursion. It likely won't cause you any problems, but if your user is really dumb and enters wrong data ~1000 times, your program will crash. Just use a loop:

def get_num() -> int:
    while True:
        raw_num = input("> ")

        try:
            return int(raw_num)

        except ValueError:
            print('Please enter a number')

In languages like Python, be careful using recursion in cases where you have no guarantees about how many times the function will recurse.

---

I'd also probably name this something closer to ask_for_num. "get" doesn't make it very clear about where the data is coming from.

---

---

Taken altogether, you'll end up with:

from typing import Generator

def collatz_gen(starting_num: int) -> Generator[int, None, None]:
    yield starting_num

    num = starting_num
    while num > 1:
        num = (num // 2) if num % 2 == 0 else (3 * num + 1)
        yield num

def ask_for_num() -> int:
    while True:
        raw_num = input("> ")

        try:
            return int(raw_num)

        except ValueError:
            print('Please enter a number')

Which can be used like:

num = ask_for_num()

for n in collatz_gen(num):
    print(n)

Answer 2

Prompt

The most obvious bad practice here is the use of a global variable. Instead of setting num as a side-effect, your function should return the result.

getNum() is not such a good name for the function. PEP 8, the official style guide for Python, says that function names should be lower_case_with_underscores. Furthermore, "get" implies that the function is retrieving a piece of data that is already stored somewhere, which is not the case here. Finally, "Num" should be more specific.

The use of recursion is not appropriate. If you want a loop, write a loop.

def ask_integer():
    """
    Return an integer entered by the user (repeatedly prompting if
    the input is not a valid integer).
    """
    while True:
        try:
            return int(input("> "))
        except ValueError:
            print("Please enter an integer")

num = ask_integer()

collatz function

Strictly speaking, you didn't follow the instructions. Your solution isn't wrong or bad — you just didn't implement the collatz function according to the specification that was given, which says that you should print and return one single number.

def collatz(num):
    """
    Given a number, print and return its successor in the Collatz sequence.
    """
    next = num // 2 if num % 2 == 0 else 3 * num + 1
    print(next)
    return next

num = ask_integer()
while num > 1:
    num = collatz(num)

Answer 3

For sake of completeness, a recursive implementation for collatz (you already got enough good suggestions for inputting num):

def collatz(num):
    print(num)
    if num == 1:
        return num
    if num % 2 == 0:
        return collatz(num // 2)
    return collatz(3 * num + 1)

collatz(3)

Outputs

3
10
5
16
8
4
2
1