reverload
reverload

Reputation: 75

Cond inside let doesn't work properly

I'm having a trouble with some Common Lisp code. I have two functions similars to the functions below:

(defun recursive-func (func lst num)
  (let ((test
          (some-func func
                     (first lst)
                     (first (rest lst))
                     num))
        (next
          (recursive-func func
                          (rest lst)
                          num)))

    (cond ((null (rest lst)) nil)
          ((null test) next)
          (t (cons test next)))))

(defun some-func (func a b num)
  (if (> a b)
      nil
      (funcall func a b num)))

When the list has only one element I want recursive-func return nil, but it doesn't and calls some-func generating a evaluation abort because b is nil. Here is a trace of execution:

CL-USER> (recursive-func #'(lambda(x y z) (+ x y z)) '(1 2 3 4 5) 5)
  0: (RECURSIVE-FUNC #<FUNCTION (LAMBDA (X Y Z)) {10035F6ABB}> (1 2 3 4 5) 5)
    1: (SOME-FUNC #<FUNCTION (LAMBDA (X Y Z)) {10035F6ABB}> 1 2 5)
    1: SOME-FUNC returned 8
    1: (RECURSIVE-FUNC #<FUNCTION (LAMBDA (X Y Z)) {10035F6ABB}> (2 3 4 5) 5)
      2: (SOME-FUNC #<FUNCTION (LAMBDA (X Y Z)) {10035F6ABB}> 2 3 5)
      2: SOME-FUNC returned 10
      2: (RECURSIVE-FUNC #<FUNCTION (LAMBDA (X Y Z)) {10035F6ABB}> (3 4 5) 5)
        3: (SOME-FUNC #<FUNCTION (LAMBDA (X Y Z)) {10035F6ABB}> 3 4 5)
        3: SOME-FUNC returned 12
        3: (RECURSIVE-FUNC #<FUNCTION (LAMBDA (X Y Z)) {10035F6ABB}> (4 5) 5)
          4: (SOME-FUNC #<FUNCTION (LAMBDA (X Y Z)) {10035F6ABB}> 4 5 5)
          4: SOME-FUNC returned 14
          4: (RECURSIVE-FUNC #<FUNCTION (LAMBDA (X Y Z)) {10035F6ABB}> (5) 5)
            5: (SOME-FUNC #<FUNCTION (LAMBDA (X Y Z)) {10035F6ABB}> 5 NIL 5)
; Evaluation aborted on #<TYPE-ERROR expected-type: NUMBER datum: NIL>.

I hope someone would can help me, thanks.

Upvotes: 3

Views: 427

Answers (1)

coredump
coredump

Reputation: 38809

The bindings in let are evaluated first, and only then you perform the tests in the cond. You need only change a bit:

(defun recursive-func (func list num)
  (if (null (rest list))
      nil
      (let ((test (some-func func
                             (first list)
                             (first (rest list))
                             num))
            (next (recursive-func func
                                  (rest list)
                                  num)))
        (cond ((null test) next)
              (t (cons test next))))))

Note that the cond could also be written:

(if test (cons test next) next)

With your example:

(recursive-func (lambda (x y z) (+ x y z))
                '(1 2 3 4 5)
                5)
=> (8 10 12 14)

Alternatively, here is how you could decompose the task (in the REPL):

> (maplist (lambda (list)
             (list
              (first list)
              (second list)))
           '(1 2 3 4 5))
=> ((1 2) (2 3) (3 4) (4 5) (5 NIL))

Prefer SECOND instead of (first (rest ...)). The last result is bound in the REPL to the variable *. Remove the last (useless) pair with BUTLAST:

(butlast *)
=> ((1 2) (2 3) (3 4) (4 5))

Then, for each couple in this list, call your function -- here it is just +. Notice the use of DESTRUCTURING-BIND:

(mapcar (lambda (list)
          (destructuring-bind (a b) list
            (+ a b 5)))
        *)
=> (8 10 12 14)

So, basically, your function can be written as:

(defun map-pair-funcall (function list number)
  (mapcar (lambda (pair)
            (destructuring-bind (a b) pair
              (funcall function a b number)))
          (butlast (maplist (lambda (list)
                              (list (first list) (second list)))
                            list))))

And thus:

(map-pair-funcall #'+ '(1 2 3 4 5) 5)
=> (8 10 12 14)

Edit:

I missed the case where the supplied function might return NIL. Wrap the form calling mapcar by (remove NIL (mapcar ...)) to filter that out.


You can perform all that in one iteration by using MAPCON. The function iterates over sublists, and concatenates the resulting lists. The function being called should thus return list, and when you return NIL, the result are simply discarded.

Let's define ensure-list (or use the one from Alexandria):

(defun ensure-list (expr)
  (if (listp expr) expr (list expr)))

The function wraps the returned value in a list, except if it is already a list (in particular, NIL). And then, the function is defined as follows:

(defun map-pair-funcall (function list number)
  (mapcon (lambda (list)
            (and (second list)
                 (ensure-list (funcall function
                                       (first list)
                                       (second list)
                                       number))))
          list))

You could also LOOP:

(defun map-pair-funcall (function list number)
  (loop 
    for (a b) on list
    for result = (and b (funcall function a b number))
    when result
      collect result))

Upvotes: 4

Related Questions