1

Note: I was trying to figure out why my while statement did not evaluate to False when the integer was, so I don't believe this is a duplicate

Doing an automating the boring stuff exercise in python where the program receives an input and reduces it to 1 using the following algorithm.

#even / 2
#odd * 3 + 1


def collatz():
    print("Enter number:")
    number = input()
    try:
        data = int(number) # Input Validation

        while int(data) != 1:
            if data % 2 == 0: #Number is even
                data = int(data/2)
                print(data)
            if data % 2 == 1: # Number is odd
                data = int(3*data+1)
                print(data)

    except:
        print("Please input a valid value")
        collatz()

collatz()

Instead of the while loop breaking, when the number is reduced to 1. The loop continues and it multiplies 1 by 3 and adds 1(Just as normal odd number). Btw many int conversions were done as I thought it may have returned a floating point.

So please tell me where are the unncessary int conversions and how to break it using a while statement. Any cleanup to code is appreciated

KOOTS HOOTS
  • 63
  • 2
  • 10

1 Answers1

2

There are a few things that you should neaten up to make your code work properly and just generally better:

  • Rename data to n, this isn't going to make much of a difference, but I would say it just makes more sense.
  • There is no need to do endless conversions to int, n only needs to be converted from a string to an integer once.
  • Don't put your entire code in a function then call it once, instead have the main algorithm in a smaller function which you then can call from the main body of your code or even create another function to call the algorithm which handles the inputting side of things.
  • Use the integer division operator (//) instead of the floating point divider (/), since the number will be even, you can be sure that the decimal place will be 0.
  • You don't need to check if n % 2 is 0 and then on the next line check if it n % 2 is 1, you can just use an if ... else clause.

And that's about it! Here's what it looks like:

#even / 2
#odd * 3 + 1

def collatz(n):
    while n != 1:
        if n % 2 == 0: #Number is even
            n = n // 2
        else:
            n = n * 3 + 1
        print(n)

number = input("Enter number:")

try:
    number = int(number)
    collatz(number)
except ValueError:
    print("Please input a valid value")

And a test shows it works (input of 24):

12
6
3
10
5
16
8
4
2
1
Joe Iddon
  • 20,101
  • 7
  • 33
  • 54
  • Thank you for all the tips and improvements! If I were to leave all the code in one function, why did my while loop not break when n or in my case data was 1? – KOOTS HOOTS Dec 30 '17 at 10:12
  • @KOOTSHOOTS Got it! It's because you are doing two `if` statements, so i the case where `data` was `2`, it would pass the first test and divide by `2` to get `1`, but then the second `if` would see it was now odd and keep it going by multiplying by `3` and adding `1`. So that is why my function is working, because I used an `else`. **:)** Please accept then! – Joe Iddon Dec 30 '17 at 11:19