patternpythonModerate
Are there any bugs or ways to make my divisor code better?
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.
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
-
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 = FalseI 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
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
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
Use
Right now, for every successfully discovered divisor, the program performs three division operations: one for
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 timeRight 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.