Common Lisp: (NULL L) Should Be A Lambda Expression

Question

I've been trying to learn Common Lisp recently, and it's the the most painful and slow learning process I've ever had with any programming langauge I've ever used. It's really annoying me. Even with Notepad++ showing what parentheses correspond to what, it's still a pain.

I've been trying to write a program that simulates a library database. I keep getting told "SYSTEM::%EXPAND-FORM: (NULL L) should be a lambda expression" - I've read other posts here on Stack Overflow that say this has to do with using too many parentheses, but after playing around with the syntax for over an hour, nothing works. I'm hoping some of you more seasonsed LISP programmers can see my hopefully novice mistake. Thank you. The code is below.

(setq library nil)

(defun add_book(bookref title author publisher)   
    (setf (get bookref 'title) title)
    (setf (get bookref 'author) author)
    (setf (get bookref 'publisher) publisher)
    (setq library (cons bookref library))
  bookref)


(defun retrieve_by (property my_value)
    (setq result nil)
    (do ((L library (cdr L)))
        (cond
            ((NULL L) result)
            (equal (get (car L) property) my_value)
                (cons (car L) result))))

I can never remember what all the parts of DO are supposed to be, and, anyway, my advice is to avoid explicit looping when you can. How about a solution in terms of FIND-IF ? See: clhs.lisp.se/Body/f_find_.htm The predicate or test function (I think you can go about it either way, both will work) will inspect the property and compare its value to the specified value. — Robert Dodier
That's a horribly poor error message! ((null l) ...) is bad syntax, because (null l) appears where a function or operator is expected. A lambda expression can be used in the operator position, but it's rare for Lisp programs to do that compared to using a function or operator! Telling the programmer that a wrong construct should be corrected to a rarely used construct is misleading. Furthermore, without explaining the context (the wrong thing appears where an operator or function name is expected) is unhelpful. — Kaz

Rainer Joswig Rainer Joswig · Accepted Answer · 2020-02-24T07:40:12

Your code better indented looks like this:

(defun retrieve_by (property my_value)
  (setq result nil)
  (do ((L library (cdr L)))
      (cond
       ((NULL L) result)
       (equal (get (car L) property) my_value)
       (cons (car L) result))))

The syntax for do is:

do ({var | (var [init-form [step-form]])}*)
   (end-test-form result-form*)
 declaration*
 {tag | statement}*

Which gives us a hint that your cond is at a wrong position and expected is a list with an end-test-form as its first element:

(defun retrieve_by (property my_value)
  (setq result nil)
  (do ((L library (cdr L)))           ; iterating L
      ((NULL L) result)               ; end test + result
     (if (equal (get (car L) property) my_value)
       (push (car L) result))))

Why did you see that error message?

"SYSTEM::%EXPAND-FORM: (NULL L) should be a lambda expression"

(do ((L library (cdr L)))           ; iterating
    (cond ((NULL L) result) ...)    ; end-test + result forms

Lisp expected a list with a test at the first position and result forms:

    (cond               ; the test is just a variable reference
     ((NULL L) result)  ; result form number one
     ...)

Now the test still looks valid, but the first result form is not:

((NULL L) result)

A list as the first element of a form is not allowed in Common Lisp - with one exception: a lambda expression:

((lambda (a b) (+ a b 10)) 12 20)

Thus your Lisp implementation complains that (null l) is not a lambda expression. Here I think error messages can be improved in implementations...

More feedback

There is an additional problem: result is an undefined variable. We need to create a local variable result. This is done with let:

(defun retrieve_by (property my_value)
  (let ((result nil))
    (do ((L library (cdr L)))
        ((NULL L) result)
      (if (equal (get (car L) property) my_value)
          (push (car L) result)))))

Another problem: library is a global variable. These are written as *library* and are defined by DEFPARAMETER or DEFVAR:

(defvar *library* nil)

(defun add_book (bookref title author publisher)   
    (setf (get bookref 'title) title)
    (setf (get bookref 'author) author)
    (setf (get bookref 'publisher) publisher)
    (setq *library* (cons bookref *library*))
  bookref)

(defun retrieve_by (property my_value)
  (let ((result nil))
    (do ((L *library* (cdr L)))
        ((null L) result)
      (if (equal (get (car L) property) my_value)
          (push (car L) result)))))

The next improvement is stylistic: code is mostly written in lower case and underscores are not used. Instead hyphens are used:

(defvar *library* nil)

(defun add-book (bookref title author publisher)   
    (setf (get bookref 'title) title)
    (setf (get bookref 'author) author)
    (setf (get bookref 'publisher) publisher)
    (setq *library* (cons bookref *library*))
  bookref)

(defun retrieve-by (property my_value)
  (let ((result nil))
    (do ((list *library* (cdr list)))
        ((null lisp) result)
      (if (equal (get (car list) property) my_value)
          (push (car list) result)))))

The next improvement is stylistic: car and cdr are old fashioned and are used for cons cell operations. If we deal with lists, we use first and rest.

This is now our code:

(defvar *library* nil)

(defun add-book (bookref title author publisher)   
  (setf (get bookref 'title)     title)
  (setf (get bookref 'author)    author)
  (setf (get bookref 'publisher) publisher)
  (push bookref *library*)
  bookref)

(defun retrieve-by (property my-value)
  (let ((result nil))
    (do ((list *library* (rest list)))
        ((null list) result)
      (if (equal (get (first list) property) my-value)
          (push (first list) result)))))

We can try it out:

CL-USER 151 > (add-book 'johann-holtrop "Johann Holtrop" "Rainald Goetz" "Suhrkamp")
JOHANN-HOLTROP

CL-USER 152 > (retrieve-by 'title "Johann Holtrop")
(JOHANN-HOLTROP)

Simplifying the code

Common Lisp has a simpler form for iterating over one list: dolist:

(defun retrieve-by (property my-value)
  (let ((result nil))
    (dolist (bookref *library* result)
      (if (equal (get bookref property) my-value)
          (push bookref result)))))

Common Lisp also has functions to find items. A strange option is to use remove with a :test-not argument. We keep all the items, which are satisfying our test, by looking at a key value we are extracting:

(defun retrieve-by (property my-value)
  (remove my-value *library*
          :test-not #'equal
          :key (lambda (bookref)
                 (get bookref property))))

Style Rules

To repeat the style rules from above:

look up the syntax of special operators using the HyperSpec (or similar)
indent your code correctly, the editor should do it for you
define your variables
global variables are written as *variable-name*
write mostly in lower case
use hyphens in compound symbols between the words: retrieve-by
often there are functions or other operators, which are easier to use than the do operator
don't use dangling parentheses
write compact code without too much whitespace lines

Common Lisp: (NULL L) Should Be A Lambda Expression

1 Answers