Lisp - "when" condition executing no matter what?

Question

I'm defining a function to test if a number is prime and I have an algorithm which works (in Python) and I have ported most of it to Lisp. The problem however is that my primality test keeps passing even when it shouldn't. For example, isPrime(13) still reaches the return NIL even though it should fail the when condition.

(defun isPrime(n)
    (cond 
        ((< n 2); numbers less than 2 aren't prime
            NIL
        )
        ((equal n 2); 2 is the only even prime
            T
        )
        ((equal (rem n 2) 0); Even numbers are never prime (besides 2)
            NIL
        )
        ((> n 2)
            (loop for i from 2 to n do(
                    when(equal (rem n i) 0);If n is evenly divisible by i, we have found a factor other than 1 and n
                        (return NIL)                   
                )
            ) 
        )
        (t T); If we get through all that with no issue, the number is prime
    )
)

Question: Why does my function reach the return NIL branch no matter what?

Also, if this is just a bad approach to testing primality, is there a more lisp-like way of doing this (not worried about performance, just algorithmic correctness and readability.)

Answer 1

First of all your code has a fairly obvious bug: once you've hit the (> n 2) case of the cond then either it's going to explicitly return nil or it will get to the end of the loop and ... implicitly return nil. The final case of the cond will never be reached.

Here is a version of it which

• is formatted using standard Lisp conventions not ones imported from C or somewhere;
• uses a conventional Lisp name for the function;
• uses better operations (compare numbers with = not equal, for instance);
• uses better bounds on the loop and a better step (no need to check even numbers, you've just done that);
• fixes the bug.

(defun primep (n)
  (cond
   ((< n 2)
    ;; numbers less than 2 are not prime
    nil)
   ((= n 2)
    ;; 2 is prime
    t)
   ((evenp n)
    ;; even numbers are not prime
    nil)
   (t
    ;; Otherwise it is a prime if no odd integer less than or equal to
    ;; its root divides it.
    (loop for i from 3 to (isqrt n) by 2
          never (zerop (rem n i))))))

However a more natural way to express this in Lisp might well be to say what you would say in English:

n is prime if it is 2 or if it is greater 2 and if it is odd, and if it has no odd divisors less than or equal to its square root.

Which we would write like this

(defun primep (n)
  (or (= n 2)                           ;2 is prime ...
      (and                              ;... otherwise ...
       (> n 2)                          ;... primes must be > 2 ...
       (oddp n)                         ;... odd ...
       ;; ... and have no odd divisorts <= their roots
       (loop for i from 3 to (isqrt n) by 2
             never (zerop (rem n i))))))

Finally you might want to check that the argument has a reasonable type: primality testing makes sense for natural numbers, so:

(defun primep (n)
  (check-type n (integer 0) "a natural number")
  (or (= n 2)                           ;2 is prime ...
      (and                              ;... otherwise ...
       (> n 2)                          ;... primes must be >= 2 ...
       (oddp n)                         ;... odd ...
       ;; ... and have no odd divisorts <= their roots
       (loop for i from 3 to (isqrt n) by 2
             never (zerop (rem n i))))))

Answer 2

Why does my function reach the return NIL branch no matter what?

Your loop form always return NIL.

See 22. LOOP for Black Belts for a good explanation of LOOP.

A loop form that does not accumulate a result or explicitly return a value returns NIL. A simple fix would be to return T after the loop, but there is a better way to express it:

(loop for i from 2 below n never (= (rem n i) 0))

Default clause

You wrote:

If we get through all that with no issue, the number is prime

This is true but it only works for tests in the cond expression. Once a guard is satisfied, like (> n 2), the value of the cond is going to be whatever value is returned by the body associated with that test. In other words, clauses don't backtrack (like in Prolog) or fall-through (like in C switches).

You could have a clause where the test could return NIL, which would make the COND try the next clause:

If there are no forms in that clause, the primary value of the test-form is returned by the cond form.

But this might be confusing.

Formatting

You should try to stick to the conventions of Lisp, and use what is known as kebab-case (dash-separated words) to name your symbols, and not leave whitespace after an open parenthesis or before a closing parenthesis.

(defun is-prime (n)
  (cond
    ...))

Personally I also like to avoid early returns when possible. In fact when possible, I find it more readable to express the result without branching (logical operators are short-circuiting, but this is a detail):

(defun is-prime (n)
  (and (> n 1)
       (or (= n 2)
           (loop
             for i from 2 to (isqrt n)
             never (= 0 (rem n i))))))

Notice that we only need to compute i up to the square root of n:

Why do we check up to the square root of a prime number to determine if it is prime?

---

edit: see also tfb's answer (start from 3 and step by 2)

Answer 3

Your code will never reach the t condition.

(> n 2) makes the execution go into the LOOP. COND will then return the value of LOOP.