0

I have written a function that returns a prime factorization of numbers n less than 20. The function uses a let to create a list of exponents, and increments those exponents when it finds that n is divisible by a prime.

In my Lisp interpreter (both gcl and clisp), when I call the following function once, I get the correct factorization but when I call it a second time, I get the sum of the factorizations of the first and second function - but isn't the scope of exponents limited to the inside of the let!? Why isn't exponents being re-assigned the value '(0 0 0 0 0 0 0 0)? How can I re-write this function so that it will withstand multiple calls?

(setf primes '(2 3 5 7 11 13 17 19))

(defun factorize (n)
  (let ((exponents '(0 0 0 0 0 0 0 0)))
    (loop for i from 0 to (- (length primes) 1) do
          (loop while (and (= (mod n (nth i primes)) 0) 
                           (not (= n 1))) do
            (incf (nth i exponents))
            (setf n (/ n (nth i primes)))))
    (return-from factorize exponents)))

Output:

>(factorize 10) ;; first time
(1 0 1 0 0 0 0 0) ;; 2^1*5*1 = 10, correct
>(factorize 10)
(2 0 2 0 0 0 0 0) ;; wrong
>(factorize 10)
(3 0 3 0 0 0 0 0)
Rainer Joswig
  • 136,269
  • 10
  • 221
  • 346
castle-bravo
  • 1,389
  • 15
  • 34
  • 1
    Quoted data is literal data. If you're familiar with C, you might recognize the behavior from literal character arrays in the code. The *reader* creates a literal list of zeros for you, and that list is stored in the compiled code, so you have only one list, and it's shared between all runs of the function. – Joshua Taylor Oct 09 '14 at 18:17

2 Answers2

2

Just another fun loop-based solution that doesn't involve random access into lists (which is rather inefficient). This also makes use of truncate, which returns a quotient and a remainder as multiple values (which you can collect with multiple-value-list and destructure with loop. Since you can iterate through the primes, you can just collect the exponents as you do so:

(defparameter *primes* '(2 3 5 7 11 13 17 19))

(defun factorize (n)
  (loop
     for p in *primes* 
     collect (loop
                for (quotient remainder) = (multiple-value-list (truncate n p))
                while (zerop remainder)
                count (setf n quotient))))


CL-USER> (factorize 10)
(1 0 1 0 0 0 0 0)
CL-USER> (factorize 12)
(2 1 0 0 0 0 0 0)
CL-USER> (factorize (* 19 19 11 5 7 2 2))
(2 0 1 1 1 0 0 2)

Of course, once you're iterating through a list and collecting a value for each other value, you could just use mapcar.

Joshua Taylor
  • 84,998
  • 9
  • 154
  • 353
1

The list '(0 0 0 0 0 0 0 0) is stored as a literal, and modifying a literal is undefined behaviour. You should use

(let ((exponents (copy-list '(0 0 0 0 0 0 0 0))))

to make a copy of the literal every time you need it, or alternatively

 (let ((exponents (list 0 0 0 0 0 0 0 0)))

Btw, use 

(defparameter *primes* '(2 3 5 7 11 13 17 19))

rather than setf at the top level. Note the * signs which are a convention to indicate that this is a special variable.

uselpa
  • 18,732
  • 2
  • 34
  • 52