Why does my Common Lisp loop only work on trivial code?

Question

I have a bit of code:

(defun divisor (x)
  (loop for i from 2 to x do
       (if (= x i) (return x) 
           (if (not (mod x i))
               (return (append i (divisor (/ x i))))))))

that should return a list of the prime factors of x. However, the code just returns x.

The defun evaluates with no errors. I've attempted to trace every function in the defun, and none is ever evaluated. Loop is a macro, so I can't trace it, but if I clear out the inside of the loop and replace with

(format t "~d " i)

it counts from 2 to x, like I would expect.

I assume I've done something wrong, but I can't figure it out.

Starkey · Accepted Answer · 2011-11-06 14:16:03Z

6

One problem is that you are using (not (mod x i)) to determine if the mod evaluation is 0, which is wrong.

Another problem is that you are appending atoms to lists and I don't think it is exactly what you want to do.

Here is a revised version that seems to work:

(defun divisor (x)
  (loop for i from 2 to x do
       (if (= x i) (return (list x))
           (if (= (mod x i) 0)
               (return (append (list i) (divisor (/ x i))))))))

answered Nov 6, 2011 at 14:16

Starkey

9,7996 gold badges34 silver badges51 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Daan Over a year ago

The reason that (not (mod x i)) doesn't work is that anything but nil is true in common lisp (including 0). Also, (cons i (divisor (/ x i)) might be a bit more concise and closer to Albertus' intent.

Albertus Over a year ago

Thanks, I did not realize that 0 is not nil. Also, append obviously needs lists, not atoms. @Daan you're right, cons is definitely what I was searching for. Thank you!

drysdam · Accepted Answer · 2011-11-06 14:13:26Z

2

In your loop, i eventually reaches x. So then you return x, discarding any other return value you might have returned from the sub-recursions.

I think you've improperly combined a loop with recursion. You only need to do one of them.

answered Nov 6, 2011 at 14:13

drysdam

8,6771 gold badge22 silver badges24 bronze badges

Comments

Patrick · Accepted Answer · 2011-11-07 11:43:34Z

1

    (defun divisor (x) 
      (loop for i from 2 to x when (eql 0 (mod x i)) collect i))

answered Nov 7, 2011 at 11:43

Patrick

5252 silver badges9 bronze badges

1 Comment

huaiyuan Over a year ago

The OP seems to want prime factorization (e.g., 12 -> 2, 2, 3), while this function returns all distinct factors (12 -> 2, 3, 4, 6); quite different.

Collectives™ on Stack Overflow

Why does my Common Lisp loop only work on trivial code?

3 Answers 3

2 Comments

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related