3
\$\begingroup\$

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.

import sys

def collatz(number):
    if number % 2 == 0: # check's if number is even.
        funNumber = number // 2
        return funNumber
    elif number % 2 == 1: # check's if number is odd.
        funNumber = 3 * number + 1
        return funNumber
        
print('Enter a number:')
try:
    inputNumber = int(input()) # check's if input is an integer.
except:
    print('Enter a number!') # if not an integer program quits.
    sys.exit()
    
whileNumber = collatz(inputNumber) # first function call

while whileNumber != 1: # while loop until function returns 1.
    print(whileNumber)
    whileNumber = collatz(whileNumber)
    if whileNumber == 1: # when number is 1 program ends.
        print(whileNumber)
        break

What do you guys think of my solution?

I'm very happy that I did it without cheating - just thinking about the problem and visualising how the code should flow.

I did get stuck on the whileNumber = collatz(inputNumber). At first, that line of code was inside the while loop but that just made my entire program freeze. Glad I got it all working.

\$\endgroup\$

3 Answers 3

Reset to default
4
\$\begingroup\$

You should not input() without a prompt argument.

You can combine your two modulus operations and the division into one divmod call.

Never bare except; in this case you're looking for ValueError.

Your input validation is correct but would be friendlier as a loop that does not call exit.

whileNumber should be while_number by PEP8.

Consider representing the sequence as an iterator function and only printing at the top level (this violates the letter, but not the spirit, of the requirements).

Suggested

from typing import Iterator


def collatz(number: int) -> int:
    quotient, is_odd = divmod(number, 2)
    if is_odd:
        return 3*number + 1
    return quotient


def collatz_sequence(start: int) -> Iterator[int]:
    number = start
    while True:
        yield number
        if number == 1:
            break
        number = collatz(number)


def get_start() -> int:
    while True:
        try:
            return int(input('Enter a number to start the Collatz sequence: '))
        except ValueError:
            pass


def main() -> None:
    start = get_start()
    for output in collatz_sequence(start):
        print(output)


if __name__ == '__main__':
    main()
\$\endgroup\$
2
  • \$\begingroup\$ def main() -> None: Could you explain what the -> does? \$\endgroup\$ Commented Jun 10, 2023 at 5:07
  • 2
    \$\begingroup\$ @JarneVercruysse It's a type hint declaring what the function will return (in this case, nothing). \$\endgroup\$ Commented Jun 10, 2023 at 13:07
5
\$\begingroup\$

According to the problem statement, the collatz() function should print the number it returns. I think that's a silly requirement, but us professional programmers sometimes have to implement silly requirements when we can't convince our customers otherwise!

Notice that the statement return funNumber is common to both branches of the if/elif. That means we can move it outside the condition. Oh, and the elif test can be simply else because any integer that's not even has to be odd:

def collatz(number):
    if number % 2 == 0: # checks if number is even.
        funNumber = number // 2
    else: # it must be odd.
        funNumber = 3 * number + 1
    print(funNumber)
    return funNumber

With the printing where we were told to put it, we don't need the if whileNumber == 1 test in the main program - we can just let the while loop do its job normally.

I recommend putting the program logic into a function, and using a main guard:

if __name__ == '__main__':
   main()

That will make it easier to reuse parts of your program as a module in the future.

\$\endgroup\$
0
2
\$\begingroup\$

I wrote a function for this many months ago.

Put everything in functions

Since you already know how to use functions, you should put everything into functions. Your while loop belongs in the Collatz function, not outside it.

Put functionality over interactivity

You use input and print in your code, this is bad, because it hinders performance. Most programmers use functions and arguments instead of interactivity.

Use return list instead of printing elements one by one.

Refactored code:

def Collatz(n: int) -> list:
    sequence = [n]
    while n != 1:
        if n % 2:
            n = 3 * n + 1
        else:
            n //= 2
        sequence.append(n)
    return sequence

Use like this: Collatz(9)

It returns:

[9, 28, 14, 7, 22, 11, 34, 17, 52, 26, 13, 40, 20, 10, 5, 16, 8, 4, 2, 1]

And now this is my actual purpose of the post: Collatz sequence works for negative numbers too. All negative numbers, when building the sequence using the Collatz rule, will inevitably fall into 3 different loops:

[
    (-2, -1),
    (-14, -7, -20, -10, -5),
    (
        -50, -25, -74, -37, -110, -55,
        -164, -82, -41, -122, -61, -182,
        -91, -272, -136, -68, -34, -17
    )
]

So I made the function to work with negative numbers as well. And I just wanted to post it.

cycles = [
    (4, 2, 1),
    (-2, -1),
    (-14, -7, -20, -10, -5),
    (
        -50, -25, -74, -37, -110, -55,
        -164, -82, -41, -122, -61, -182,
        -91, -272, -136, -68, -34, -17
    )
]
loops = {term: len(cycle) for cycle in cycles for term in cycle}
stops = {term: cycle[-1] for cycle in cycles for term in cycle}

def Collatz(n):
    if not n:
        raise ValueError('n should not be 0')
    
    sequence = [n]
    loop_iter = 0
    looping = False
    while True:
        if not looping and n in loops:
            loop_terms = loops[n]
            looping = True
        
        if looping:
            loop_iter += 1
            if loop_iter == loop_terms:
                break
        
        if n % 2:
            n = 3 * n + 1
        else:
            n //= 2
        
        sequence.append(n)
    
    return sequence

It works with every integer, except zero, because if you start with 0 you can never reach anything other than 0 according to the rules.

In [22]: Collatz(-9)
Out[22]: [-9, -26, -13, -38, -19, -56, -28, -14, -7, -20, -10, -5]
\$\endgroup\$
2
  • \$\begingroup\$ You establish stops but terminate when the count of "loop values" seen mathes the loop's length. Is this to ensure the entire loop gets appended? \$\endgroup\$ Commented Jan 27 at 4:50
  • \$\begingroup\$ @greybeard Yes, I did that to ensure the entire loop gets appended exactly once, so that the code terminates with all relevant information included. And to prevent the code from running forever. \$\endgroup\$ Commented Jan 27 at 6:58

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.