HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonModerate

Are there any bugs or ways to make my divisor code better?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
arewaysanymakebettercodebugstheredivisor

Problem

After coding my divisor code which I did as a practice for my GCSE stuff which I have to take soon, I have decided to ask the community if there are any bugs or ways I can improve my code.

import time, math
from array import *
def programme():
    num = input("Enter your Number: ")
    try:
        intnum = int(num)
        if(intnum < 0):
            error("POSITIVE NUMBERS ONLY")
        else:
            numbers = array("i",[])
            starttimer = time.time()
            i = 1
            print("\nThe divisors of your number are:"),
            while i <= math.sqrt(intnum):
                if (intnum % i) == 0:
                    numbers.insert(0,i)
                    numbers.insert(0,int(intnum/i))
                    print(i,":", int(intnum/i))
                i += 1
            numbers = sorted(numbers, reverse = True)
            print("The Highest Proper Divisor Is\n--------\n",str(numbers[1]) , "\n--------\nIt took"  ,round(time.time() - starttimer, 2)  ,"seconds to finish finding the divisors." )
    except ValueError:
       error("NOT VALID NUMBER")
    except OverflowError:
       error("NUMBER IS TO LARGE")
    except:
       error("UNKNOWN ERROR")

def error(error):
    print("\n----------------------------------\nERROR - " + error + "\n----------------------------------\n")
running = True
while(running):
    programme()
    print("----------------------------------")
    restart = input("Do You Want Restart?")
    restart = restart.lower()
    if restart in ("yes", "y", "ok", "sure", ""):
        print("Restarting\n----------------------------------")
    else:
        print("closing Down")
        running = False


I don't care how small the improvement is all I want are ways in which I can make it better!

When inputting 262144 I get

```
Enter your Number: 262144

The divisors of your number are:
1 : 262144
2 : 131072
4 : 65536
8 : 32768
16 : 16384
32 : 8192
64 : 4096
128 : 2048
256 : 1024
512 : 512
The Highest Proper Divisor Is
-

Solution

Here are some thoughts on this code:

Chose descriptive function names

The name programme is not really very descriptive of what the function does. You might call it something like factor to give the reader of your code a better idea of what it actually does.

Document your code

Python supports the use of multiple different kinds of comments, including ones that can be made into unit tests. Get in the habit of commenting your code carefully.

Separate I/O from calculation

The main purpose of the program is to factor numbers into divisors which is something that is potentially reusable. Both to make it more clear as to what the program is doing and to allow for future re-use, I'd suggest extracting the factoring part into a separate function and the have the input from the user be done within the main routine or some other input-only function.

Separate timing from the thing being timed

On my machine, the timer always reports 0.0 seconds, which makes it hard to compare the effects of changes in the code. For that reason, what's often done is to call the routine to be timed multiple times to get better resolution. In order to do that, you'd have to remove the timing from the routine you're timing, which is good design practice anyway. Right now, the print function is part of the code that's being timed.

Think of your user

Right now, the user has to enter "yes" or the equivalent and then enter another number to be factored. However, the prompt doesn't suggest that "y" is a valid answer. Adding that to the prompt would help the user understand what the computer is expecting. Better still, would be t eliminate that question and to simply ask for the next number with a number '0' being the user's way to specify no more numbers. Of course the prompt should be changed to tell the user of this fact.

Expand the range

Right now, the size of numbers that can be factored is limited to what will fit within an int, but it would be much more interesting to be able to factor really large numbers.

Use divmod() to save some time

Right now, for every successfully discovered divisor, the program performs three division operations: one for %, and then two more to insert the number into numbers and again to print. Instead, you could do the division once using the divmod function which gives you both the quotient and remainder at the same time.

Context

StackExchange Code Review Q#48328, answer score: 12

Revisions (0)

No revisions yet.