From: Mark Cox
Subject: macro design help for open style if-exists and if-does-not-exist style functions
Date: 
Message-ID: <45ecc2c7$1@news.qut.edu.au>
To comp.lang.lisp,

I find myself writing functions which take keyword parameters which 
automatically setup restart bindings to handle errors. An example within 
the CLHS is the function open: 
http://www.lispworks.com/documentation/HyperSpec/Body/f_open.htm.

For a trivial example, take inserting an element into a set.

(defun insert-value-into-set (set value &key (if-exists :error)))

Now the default for this function (i.e. (eql if-exists :error)) is to 
signal an error that has a number of restarts available to it:
1. overwrite		Remove the existing value and replace it with
			the given parameter.
2. use-existing		Leave the existing value where it is.
3. use-value		Ask for a new value to insert

As you have probably guessed by now but I'll include for completeness, 
if-exists could also take the values :overwrite or :use-existing in 
which it automatically performs the appropriate restarts. I'm not too 
sure about specifying a :use-value since it is interactive.

The most succinct way I've found to implement these types of functions 
is to do the following:

; shamefully copied and pasted from the CLHS
(defun read-new-value ()
   (format t "Enter a new value: ")
   (multiple-value-list (eval (read))))

(defun insert-value-into-set (set value &key (if-exists :error))
   (tagbody
    test-for-item
      (if (find value set)
	 (handler-bind
	     ((error
	       (lambda (cnd)
		 (ecase if-exists
		   (:overwrite (invoke-restart 'overwrite))
		   (:use-existing (invoke-restart 'use-existing))
		   (:error nil)))))
	   (restart-case (error (format nil "Found duplicate key ~A."
				        value))
	     (overwrite ()
	       (setf set (delete value set)))
	     (use-existing ()
	       (return-from insert-value-into-set set))
	     (use-value (new-value) :interactive read-new-value
			(setf value new-value)
			(go test-for-item)))))
      (push value set))
   set)

As can be seen, you know what values of if-exists can take a priori so a 
macro could fill in the contents of the handler-bind coupled with the 
restart-case for you.

I'm relatively new to lisp, so I'm not quite sure whether these types of 
functions are good lisp practice, but I like the concept and I find 
myself writing these types of functions a fair bit, hence the need for a 
macro to alleviate some of the typing.

The macro as I see it generates the handler-bind call for a mapping of 
:if-exists parameters to restarts (which could also be autogenerated 
from the keyword symbol name). e.g.

(parameterized-handler-bind
     ; 1. the error to catch,
     ; 2. the parameter to determine the restart
     ; 3. The fallback error, i.e. regenerate
     ; 4. List of keywords to signal mappings.
    ((error if-exists :error (:overwrite (:use-existing 'use-existing))))
       (restart-case ....))

Ah the point of this post! As I've got tunnel vision with respect to the 
problem, I thought I'd go out on a limb and get some feedback from more 
experienced lispers such as yourselves. The thing I love about lisp is 
that it makes you think and ask questions. So here are mine:

Is it good practice doing these types of functions? Is there a better 
way of doing it?
Are there cases in which the pattern won't work? what assumptions am I 
making?
I haven't had a programming problem in which the solution involved an 
interactive restart being called and would require an :if-exists option, 
other than when in the debugger, so if someone could give some insight 
into anything to do with that, it would be appreciated.

Thanks
Mark
From: Madhu
Subject: Re: macro design help for open style if-exists and if-does-not-exist style functions
Date: 
Message-ID: <m3r6s24qq2.fsf@robolove.meer.net>
* Mark Cox <··········@news.qut.edu.au> :
| (defun insert-value-into-set (set value &key (if-exists :error)))
|
| Now the default for this function (i.e. (eql if-exists :error)) is to 
| signal an error that has a number of restarts available to it:
| 1. overwrite		Remove the existing value and replace it with
| 			the given parameter.
| 2. use-existing		Leave the existing value where it is.
| 3. use-value		Ask for a new value to insert
|
| As you have probably guessed by now but I'll include for completeness, 
| if-exists could also take the values :overwrite or :use-existing in 
| which it automatically performs the appropriate restarts. I'm not too 
| sure about specifying a :use-value since it is interactive.

I think this is a bad example, in light of the errors pointed out
below.  Secondly, I believe that no real good can come from conflating
keyword arguments (that specify the function's behaviour) with
restarts (that handle an exceptional situation).

| The most succinct way I've found to implement these types of functions 
| is to do the following:
|
| ; shamefully copied and pasted from the CLHS
| (defun read-new-value ()
|    (format t "Enter a new value: ")
|    (multiple-value-list (eval (read))))
|
| (defun insert-value-into-set (set value &key (if-exists :error))
|    (tagbody
|     test-for-item
|       (if (find value set)
| 	 (handler-bind
| 	     ((error
| 	       (lambda (cnd)
| 		 (ecase if-exists
| 		   (:overwrite (invoke-restart 'overwrite))
| 		   (:use-existing (invoke-restart 'use-existing))
| 		   (:error nil)))))

This looks like an error. The RESTART-CASE should enclose the
HANDLER-BIND for your stated purpose. The way it is shown here, the
restarts wont be visible.

| 	   (restart-case (error (format nil "Found duplicate key ~A."
| 				        value))
| 	     (overwrite ()
| 	       (setf set (delete value set)))

This looks like an error. :overwrite is deleting the element. Your
text spec above makes it entirely equivalent to USE-EXISTING.

| 	     (use-existing ()
| 	       (return-from insert-value-into-set set))
| 	     (use-value (new-value) :interactive read-new-value
| 			(setf value new-value)
| 			(go test-for-item)))))
|       (push value set))

This looks like another error. VALUE is pushed to the list named by
SET. The caller's list is NOT MUTATED. This is probably not what you
want, but I'll assume this is just a contrived example. (This means
that INSERT-VALUE-INTO-SET should be a macro if SET is a place)


|    set)

The whole thing looks like it is using the condition system for
glorified control flow. I'd prefer to see something like:

(define-condition duplicate-key () 
  ((set :initarg :set :reader duplicate-key-list)
   (value :initarg :value :reader duplicate-key-key))
  (:report ...))

(defun insert-value-into-set (set value &key (if-exists :error))
  (if (find value set)
      (ecase if-exists 
	(:overwrite) ; XXX
	(:use-existing)
	(:error (error 'duplicate-value :set set :value value)))
      (error "TODO: Unimplemented")))


Note that the call to INSERT-VALUE-INTO-SET one specifies the
:if-exists keyword argument. The only time the condition system needs
to be invoked is if :if-exists is :error.  (Ane you only need use
handler bind if you want to handle the condition programatically.)


Now assuming you want to handle the duplicate-key exception
interactively, (again, Note THIS SKETCH IS IS A BAD EXAMPLE, as far as
granularity of the exception-producing code goes!)

(loop
 (restart-case (INSERT-VALUE-INTO-SET SET VALUE) ; throws duplicate-key
   (use-value (new-value) :interactive read-new-value
     (setf value new-value)
     (go loop))
   (use-existing () 
     (return))))


But, to answer your question, personally I think a macro is not
appropriate --- if you do not conflate function behaviour with
exception handling.

--
Madhu