2

This Python function outputs a list based on the collatz conjecture. This is an unsolved maths problem where the function will perform different operations on 'n' depending on if it is odd or even, outputting 'n' to a list called 'seq' each time the function repeats. Eventually 'n' will decay to the end point ('1') once the number '16' appears in the 'chain'.

I am trying to make the code as concise as possible here. Is there any way to shorten the function?

This is my newbie Python code:

def collatz(n):

    seq = []
    n = int(n)

    if n == 0:
        return 
    
    elif n == 1:
        return seq + [n]
    
    elif n > 1 == True and n % 2 == 0:
        return seq + [n] + collatz(n/2)
    
    else:
        return seq + [n] + collatz(3*n+1)

print(collatz(7))

this outputs

[7, 22, 11, 34, 17, 52, 26, 13, 40, 20, 10, 5, 16, 8, 4, 2, 1]
Andrej Kesely
  • 168,389
  • 15
  • 48
  • 91
Alastair909
  • 29
  • 4
  • Yes, you can remove `seq = []` and all other references to `seq`. That variable does nothing in your function. – quamrana Mar 26 '23 at 12:04

2 Answers2

3

As of right now, the function you gave isn't fully robust for negative inputs and 0: I would rewrite it as such below. I think yours is very readable though, and I wouldn't promote more conciseness: readability > conciseness almost always.

def collatz(n):

    seq = []
    n = int(n)

    if n <= 0:
        return False
    
    elif n == 1:
        return seq + [n]
    
    elif n % 2 == 0:
        return seq + [n] + collatz(n/2)
    
    else:
        return seq + [n] + collatz(3*n+1)

print(collatz(7))

If you really want more concise code, consider this solution by @Kelly Bundy:

def collatz(n):
    return [1] if n == 1 else [n] + collatz(3*n+1 if n%2 else n//2)

print(collatz(7))

Alternatively, an iterative rather than recursive solution

def collatz(n):
    seq = [n]
    while n != 1:
        n = 3*n+1 if n%2 else n//2
        seq.append(n)
    return seq

Sanitise your inputs as you wish.

Jakub Skop
  • 137
  • 7
  • Except that the if should be: `if n <= 0:`, otherwise the returned list doesn't contain a final `1`. – quamrana Mar 26 '23 at 12:09
  • Why the hyper-complicated recursive call? – Kelly Bundy Mar 26 '23 at 12:22
  • Especially since you talk about readability :-) – Kelly Bundy Mar 26 '23 at 12:24
  • @KellyBundy That's a rather good point - The original used recursion and I decided to follow suite, but of course iterative solutions are much more robust for large inputs, and faster too. I have added an iterative solution too :) – Jakub Skop Mar 26 '23 at 12:27
  • Actually I didn't mean the recursion but the expression you use to compute the next n value there. I mean, I'd write it as `def collatz(n): return [1] if n == 1 else [n] + collatz(3*n+1 if n%2 else n//2)`. – Kelly Bundy Mar 26 '23 at 12:40
  • @KellyBundy Had no idea you could write python if statements like that inside the function call. I guess you learn something new everyday. – Jakub Skop Mar 26 '23 at 12:44
  • 2
    It's not an If statement. You indeed can't write statements inside expressions. Only expressions. And that's a [conditional expression](https://docs.python.org/3/reference/expressions.html#conditional-expressions). – Kelly Bundy Mar 26 '23 at 12:59
0

I've got issues with the more readable version of the currently selected answer. First, there's no need for the else and elif statements given the return statements.

Second, I wouldn't endorse a function that returns a boolean sometimes and a list otherwise. The return type should be consistent:

def collatz(n):
    if n <= 0:
        return []

    if n == 1:
        return [n]

    if n % 2:
        return [n, *collatz(3 * n + 1)]

    return [n, *collatz(n // 2)]

print(collatz(7))
cdlane
  • 40,441
  • 5
  • 32
  • 81