Automate the Boring Stuff with Python - The Collatz sequence project

Question

I am new to programming and started learning with Automate the Boring Stuff with Python. I just completed chapter 3's "Collatz sequence" project.

I was wondering what would you do differently and why? Trying to learn how to think like a programmer and become more efficient.

def collatz(number):
    global nextNumber
    if number % 2 == 0:
        nextNumber = (number//2)
        print(nextNumber)
    else:
        nextNumber = (3*number+1)
        print(nextNumber)

print('Type in an integer.')
integer=input()

if integer.isdigit() ==True:
    collatz(int(integer))
    while nextNumber !=1:
        collatz(nextNumber)

while integer.isdigit() != True :
        print('Please try again.')
        integer=input()
        collatz(int(integer))
        while nextNumber !=1:
            collatz(nextNumber)

print('Thank you!')

Answer 1

One thing you’ll want to learn is not to repeat yourself, when coding.

Consider:

def collatz(number):
    global nextNumber
    if number % 2 == 0:
        nextNumber = number//2
        print(nextNumber)
    else:
        nextNumber = 3*number+1
        print(nextNumber)

You have two identical print statements at the end of both the if and the else clauses. These can be combined and moved out of the if-else:

def collatz(number):
    global nextNumber
    if number % 2 == 0:
        nextNumber = number//2
    else:
        nextNumber = 3*number+1
    print(nextNumber)

You are printing in your collatz generator function. If you want the longest sequence starting under 10000000, you’re going to do a lot of printing! Instead, move the printing responsibility to the caller. They know whether they want to print it, or just find the length.

Don’t use global to return one value from a function. (As Aaron states in the comments: "don't use global ever if you can avoid it (which is always)"). Just return the value,

def collatz(number):
    if number % 2 == 0:
        nextNumber = number//2
    else:
        nextNumber = 3*number+1
    return nextNumber

and let the caller assign it to whatever variable they want.

nextNumber = collatz( int(number) )
print(nextNumber)
while nextNumber != 1:
    nextNumber = collatz(nextNumber)
    print(nextNumber)

Reordering, to remove one of the calls to collatz:

number = int(number)
while number != 1:
    number = collatz( number )
    print(number)

Here, you are try to validate the user input is a number:

if integer.isdigit() ==True:
    collatz(int(integer))
    # ... print Collatz sequence ...

while integer.isdigit() != True :
        print('Please try again.')
        integer=input()
        # ... print Collatz sequence ...

Multiple issues here.

1. You are using an if <expr> followed by a while <not expr> to perform a sort of if ... else ... statement. If the first input is valid, you print the Collatz sequence in the "then" clause, and the while loop never starts because the condition is immediately False. If the if <expr> doesn't pass, the while <not expr> catches control and prompts the user for valid input, in a loop.
2. You are repeating the code to print that Collatz sequence. And as I said above, (omitted, because *I* am *not* going to repeat myself).
3. The while loop will fail with a ValueError exception at collatz(int(integer)) if the user enters an invalid input the second time.
4. .isdigit() tests if the string consists only of "digit" characters, including odd things like the superscript 2 (²), which are not valid for the int(...) method. So you could still get a ValueError exception even when .isdigit() returns True! You would need to use .isdecimal() instead.

These issues can be avoiding by using Python's exceptions. Try to convert the input to an integer, and use the exception to detect invalid input. Python’s exception handling is exceptional! Learn it. Rely on it:

number = None
while number is None:
    try:
        number = int( input("Enter a number: ") )
    except ValueError:
        print("Try again")

 # ... print collatz sequence here

Answer 2

@AJNeufeld's answer is great. One thing to add:

Boolean tests should never have things like actuallyabool == True. If you think about how the interpreter sees that, it looks like if True == True. So, instead of

if integer.isdigit() == True:

Just

if integer.isdigit():