I'm trying to learn Python through the excellent online book Dive Into Python 3. I have just finished chapter 2 about data types and felt like trying to write a program on my own.
The program takes an integer as input and factorizes it into prime numbers:
$ python main.py 6144 6144 -> (2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 3) $ python main.py 253789134856 253789134856 -> (2, 2, 2, 523, 60657059)
I'd like to know if I could improve it in any way. Or if I've used any bad practice and so on.
#!/usr/bin/python import sys import math def prime_factorize(n): factors =  number = math.fabs(n) while number > 1: factor = get_next_prime_factor(number) factors.append(factor) number /= factor if n < -1: # If we'd check for < 0, -1 would give us trouble factors = -factors return tuple(factors) def get_next_prime_factor(n): if n % 2 == 0: return 2 # Not 'good' [also] checking non-prime numbers I guess? # But the alternative, creating a list of prime numbers, # wouldn't it be more demanding? Process of creating it. for x in range(3, int(math.ceil(math.sqrt(n)) + 1), 2): if n % x == 0: return x return int(n) if __name__ == "__main__": if len(sys.argv) != 2: print("Usage: %s <integer>" % sys.argv) exit() try: number = int(sys.argv) except ValueError: print("'%s' is not an integer!" % sys.argv) else: print("%d -> %s" % (number, prime_factorize(number)))
@JeffMercado: Interesting link as I haven't encountered this before. And yes, the memoization technique should be easy to implement in my case since it is basically the same as the Fibonacci example.
In the Fibonacci example, they have a map (dictionary in Python?) outside the function, but should I do that? Global variables “are bad”? If that's the case, where does it belong? In the function prime_factorize() and I pass the map as an argument? I'm having a hard time deciding things like this, but I guess it gets easier with experience...
Prime factorization really only makes sense for integers. So you really use abs not fabs.
I couldn't find abs when I ran help(math), only fabs. I thought fabs was my only choice but I just found out that abs doesn't even reside in the math module. Fixed.
Tuples are for heterogeneous data. Keep this a list. It is conceptually a list of numbers, and it so it should be stored in a python list not a tuple.
You write that tuples are for heterogeneous data. I searched SO and many seems to be of this opinion. However, in the book I'm following, the author gives a few points for using a tuple:
Tuples are faster than lists. If you’re defining a constant set of values and all you’re ever going to do with it is iterate through it, use a tuple instead of a list.
It makes your code safer if you “write-protect” data that doesn’t need to be changed. Using a tuple instead of a list is like having an implied assert statement that shows this data is constant, and that special thought (and a specific function) is required to override that.
That is why a returned a tuple. Is the author's points valid or just not in this case?
Your expression, int(math.ceil(math.sqrt(n)) + 1) seems to be more complicated that it needs to be. Couldn't you get by with int(math.sqrt(n) + 1).
The upper value of range was unnecessarily complicated. Fixed.
Again, why are you trying to support something that isn't an int?
I was returning int(n) from get_next_prime_factor(n) since the number that is passed in to the function becomes a float when I divide it (in prime_factorize), so if I return just n from the function, I return a float which gets added to the list 'factors'. When I then print the factors, I get e.g. '11.0' as the last factor for the number 88.
Any other way to fix that?
I'd pass an exit code to indicate failure
Should I just exit(1) or something like that?