CODEWITHSUNDEEP
functional-programmingcommon-lispsbcl
EliSquared
EliSquared

Reputation: 1559

LISP Appropriate Way to Return Value From Function

So I am working my way through Paul Graham's Common Lisp and one questions asks to create a union function that maintains the order of the elements in the lists being unionized. To that end I have written the following function:

(defun new-union (listA listB)
  (setq retset (list (car listA)))
  (loop for el in (append (cdr listA) listB)
    do (if (not(member el retset))
      (push el (cdr (last retset)))))
  (return-from new-union retset))

This returns the unique elements of each list while maintaining the order so if I create and run:

(setq listA '(a b c a))
(setq listB '(c d e))
(new-union listA listB)

The return is:

(A B C D E)

So the first things is I get a compiler warning: "undefined variable: RETSET" on this line: (setq retset (list (car listA))). The other thing is that the above method seems to be a more "object-oriented" way of doing things rather than the LISP way with the return-from statement.

Is it possible to write this code in a more "lisp-appropriate" manner without the compiler error?

*Edit: using the answer from @Sylwester I have rewritten the function as follows and get no errors:

(defun new-union (listA listB)
 (let ((retset (list (car listA))))
   (loop for el in (append (cdr listA) listB)
         do (if (not (member el retset))
              (push el (cdr (last retset)))))
   retset))

Upvotes: 1

Views: 4229

Answers (2)

Rainer Joswig
Rainer Joswig

Reputation: 139411

A slightly better list-based version:

  • union for more than two lists
  • does not use LAST
  • still uses MEMBER
  • does not use APPEND
  • DOLIST returns the value RETSET

Code

(defun new-union (&rest lists
                  &aux (retset (list (caar lists)))
                       (rretset retset))
  (dolist (list lists retset)
    (dolist (el list)
      (unless (member el retset)
        (setf (cdr rretset) (list el)
              rretset (cdr rretset))))))

Upvotes: 3

Sylwester
Sylwester

Reputation: 48775

setq is to update an existing binding and your variable retset is not created. How this is handled is not specified in the standard so you cannot depend on code that touches it. You make global variables with defparameter and defvar while you can make local variables with &aux in functions, let and loop can create variables with with. Thus:

(defun new-union (list-a list-b)
  (let ((retset (list (car list-a))))
    ...
    retset
    ))

Is the same as this using &aux:

(defun new-union (list-a list-b &aux (retset (list (car list-a))))
  ...
  retset
  )

And also the same as loop with clause:

(defun new-union (list-a list-b)
  (loop :with retset := (list (car list-a))
     ...
     :finally (return retset)))

About return values. In tail position the value evaluated is the returned value. eg. 

(if (< 3 4)
    8
    10)

Here 8 is returned. That means (return from new-union retset) in your code, which is in tail position, could have been written just retset.

Now if you have code which is not in tail position and you wish to return early you can do what you did in tail position and it will work.

The one I use (return retset) returns from the nearest unnamed (nil) block while return-from returns from a named block. loop has the keyword named which allows you to choose the name of the block it produces.

Asking a lisper to implement such a trivial function you will get lots of answers. With the specifications and tests you had I would have done:

(defun new-union (&rest lists &aux (hash (make-hash-table :test 'equal)))
  (loop :for list :in lists
        :nconc (loop :for element :in list 
                     :if (gethash element hash t)
                         :collect element
                         :do (setf (gethash element hash) nil))))

Upvotes: 6

Related Questions