patternpythonMinor
Printing all prime factors of a number input by the user
Viewed 0 times
factorsnumberthealluserinputprintingprime
Problem
I just started teaching myself to code a few days ago using the Euler problems. I finished a variant of problem 3: print the prime factors of a number input by the user. I just want feedback and code cleaning from someone who knows better than I do to help me progress.
import math
#What is the largest prime factor of the number 600851475143?
n=raw_input('Enter the integer you wish to find the prime factors of: ')
n = int(float(n))
L=[]
def too_small():
"""to be called for int 0:
L.append(1)
return L
if n 4
L = too_small()
print L
exit()
if n == 4: #because I'm lazy
L.append(2)
L.append(2)
print L
exit()
def primefactor():
"""prints list of all prime factors for int > 4"""
test=True
b=n
while(test):
i=2
while((b%i!=0)&(i=(b/2): #number is prime
L.append(b)
L.append(1)
test=False
else: #number is not prime
a=(b/i)
L.append(i)
b=a
return L
L = primefactor()
print L
exit()Solution
Consistency
Why is there sometimes a
Lint
PEP 8, the official style guide, specifies 4 spaces per level of indentation. This is a strong convention for Python, where whitespace is significant.
Also, by PEP 8,
It makes no sense to convert
Some of your comments are unclear. For example:
… would be better written as
Since this is all integer arithmetic, you can prepare your code for use with Python 3 by using the
Counting loops in Python are better written using some kind of
Generality and organization
Why doesn't
Flag variables, especially poorly named ones like
First rewrite
Incorporating the remarks above, and more…
Yielding results
You can stream the results back to the caller by
As an optimization, you can avoid considering even factors other than 2.
Why is there sometimes a
1 at the end of the results? Should it even be there, considering that 1 is not a prime number? If you do include it in the results, why is it at the end instead of at the beginning of the list?Enter the integer you wish to find the prime factors of: 60
[2, 2, 3, 5, 1]
Enter the integer you wish to find the prime factors of: 5
[5, 1]
Enter the integer you wish to find the prime factors of: 4
[2, 2]Lint
PEP 8, the official style guide, specifies 4 spaces per level of indentation. This is a strong convention for Python, where whitespace is significant.
Also, by PEP 8,
primefactor() would be better named prime_factors().It makes no sense to convert
n to a float, then convert it again to an int.import math is pointless; you never use anything in the math module.Some of your comments are unclear. For example:
if i>=(b/2): #number is prime… would be better written as
# b is prime.Since this is all integer arithmetic, you can prepare your code for use with Python 3 by using the
// operator rather than /.Counting loops in Python are better written using some kind of
for loop in conjunction with an iterator. Here, I recommend itertools.count(2).Generality and organization
primefactor() would be better as a function that accepts a parameter, rather than using the global variable n. It should definitely return a list that it created, rather than modifying a global L as a side-effect.Why doesn't
primefactor() also work for n ≤ 4? If you organized the code properly, the caller wouldn't be responsible for handling special cases (which aren't even necessary in the first place). There would be no need to call exit() — which you should almost never need in a properly structured program.Flag variables, especially poorly named ones like
test, are rarely desirable or necessary. Here, you can easily eliminate the test variable by changing test=False to return L.First rewrite
Incorporating the remarks above, and more…
from itertools import count
def prime_factors(n):
factors = []
for i in count(2):
while n % i == 0:
factors.append(i)
n //= i
if 2 * i > n:
return factors
n = int(raw_input('Enter the integer you wish to find the prime factors of: '))
print(prime_factors(n))Yielding results
You can stream the results back to the caller by
yielding prime factors as you find them, rather than building the entire list.As an optimization, you can avoid considering even factors other than 2.
from itertools import chain, count
def prime_factors(n):
for i in chain([2], count(3, 2)): # Try 2, 3, 5, 7, 9, 11, 13, 15...
while n % i == 0:
yield i
n //= i
if 2 * i > n:
return
n = int(raw_input('Enter the integer you wish to find the prime factors of: '))
print(', '.join(prime_factors(n)))Code Snippets
Enter the integer you wish to find the prime factors of: 60
[2, 2, 3, 5, 1]
Enter the integer you wish to find the prime factors of: 5
[5, 1]
Enter the integer you wish to find the prime factors of: 4
[2, 2]if i>=(b/2): #number is primefrom itertools import count
def prime_factors(n):
factors = []
for i in count(2):
while n % i == 0:
factors.append(i)
n //= i
if 2 * i > n:
return factors
n = int(raw_input('Enter the integer you wish to find the prime factors of: '))
print(prime_factors(n))from itertools import chain, count
def prime_factors(n):
for i in chain([2], count(3, 2)): # Try 2, 3, 5, 7, 9, 11, 13, 15...
while n % i == 0:
yield i
n //= i
if 2 * i > n:
return
n = int(raw_input('Enter the integer you wish to find the prime factors of: '))
print(', '.join(prime_factors(n)))Context
StackExchange Code Review Q#129215, answer score: 3
Revisions (0)
No revisions yet.