From: Stefan Arentz
Subject: Macro problem
Date: 
Message-ID: <87hcypxtiq.fsf@kip.sateh.com>
I'm trying to do a little variation on the CD database from Practical Common Lisp.

I thought it would be a good excercise to let update-cds:

(defun update-cds (selector-fn &key title artist rating (ripped nil ripped-p))
  (setf *db*
        (mapcar
         (lambda (row)
           (when (funcall selector-fn row)
             (if title (setf (getf row :title) title))
             (if artist (setf (getf row :artist) artist))
             (if rating (setf (getf row :rating) rating))
             (if ripped-p (setf (getf row :ripped) ripped))) row)
         *db*)))

use a macro too so that I can get rid of all the if statements. After all when
update-cds is called like this:

 (update-cds (where :artist "Tool") :rating 2)

i already know that I only have to update the rating property.

So I did this:

 (defun make-update-expr (field value)
   `(progn (format t "Updating ~a to ~a" ,field ,value) (setf (getf cd ,field) ,value)))

 (defun make-updates-list (fields)
   (loop while fields
         collecting (make-update-expr (pop fields) (pop fields))))

 (defmacro update (&rest clauses)
   `(lambda (cd) ,@(make-updates-list clauses)))

 (defun update-cds2 (selector-fn &rest update-clauses)
   (let ((updater-fn (update update-clauses)))
     (setf *db*
           (mapcar
            (lambda (row)
              (when (funcall selector-fn row)
                (funcall updater-fn row))
              row)
            *db*))))

but the result is rather interesting:

 Updating (RATING 2) to NIL
 Updating (RATING 2) to NIL

it seems that the update-clauses are passed to update as a list and not
as the individual &rest items.

I know the code is otherwise ok because if I do this:

 (defun update-cds2 (selector-fn &rest update-clauses)
   (let ((updater-fn (update :rating 10 :ripped nil)))
   ...

The cds are correctly updated.

 S.

From: Ken Tilton
Subject: Re: Macro problem
Date: 
Message-ID: <WCwTg.677$hM7.529@newsfe12.lga>
Stefan Arentz wrote:
> I'm trying to do a little variation on the CD database from Practical Common Lisp.
> 
> I thought it would be a good excercise to let update-cds:
> 
> (defun update-cds (selector-fn &key title artist rating (ripped nil ripped-p))
>   (setf *db*
>         (mapcar
>          (lambda (row)
>            (when (funcall selector-fn row)
>              (if title (setf (getf row :title) title))
>              (if artist (setf (getf row :artist) artist))
>              (if rating (setf (getf row :rating) rating))
>              (if ripped-p (setf (getf row :ripped) ripped))) row)
>          *db*)))
> 
> use a macro too so that I can get rid of all the if statements. After all when
> update-cds is called like this:
> 
>  (update-cds (where :artist "Tool") :rating 2)
> 
> i already know that I only have to update the rating property.
> 
> So I did this:
> 
>  (defun make-update-expr (field value)
>    `(progn (format t "Updating ~a to ~a" ,field ,value) (setf (getf cd ,field) ,value)))
> 
>  (defun make-updates-list (fields)
>    (loop while fields
>          collecting (make-update-expr (pop fields) (pop fields))))
> 
>  (defmacro update (&rest clauses)
>    `(lambda (cd) ,@(make-updates-list clauses)))
> 
>  (defun update-cds2 (selector-fn &rest update-clauses)
>    (let ((updater-fn (update update-clauses)))
>      (setf *db*
>            (mapcar
>             (lambda (row)
>               (when (funcall selector-fn row)
>                 (funcall updater-fn row))
>               row)
>             *db*))))
> 
> but the result is rather interesting:
> 
>  Updating (RATING 2) to NIL
>  Updating (RATING 2) to NIL
> 
> it seems that the update-clauses are passed to update as a list and not
> as the individual &rest items.

Put some print statements in your macros to see what the input and 
output is, and then recompile update-cds2 to see what is going on. 
Should be an eye-opener.

hth, kt

-- 
Cells: http://common-lisp.net/project/cells/

"I'll say I'm losing my grip, and it feels terrific."
    -- Smiling husband to scowling wife, New Yorker cartoon
From: Ken Tilton
Subject: Re: Macro problem
Date: 
Message-ID: <PUwTg.683$hM7.605@newsfe12.lga>
Ken Tilton wrote:
> 
> 
> Stefan Arentz wrote:
> 
>> I'm trying to do a little variation on the CD database from Practical 
>> Common Lisp.
>>
>> I thought it would be a good excercise to let update-cds:
>>
>> (defun update-cds (selector-fn &key title artist rating (ripped nil 
>> ripped-p))
>>   (setf *db*
>>         (mapcar
>>          (lambda (row)
>>            (when (funcall selector-fn row)
>>              (if title (setf (getf row :title) title))
>>              (if artist (setf (getf row :artist) artist))
>>              (if rating (setf (getf row :rating) rating))
>>              (if ripped-p (setf (getf row :ripped) ripped))) row)
>>          *db*)))
>>
>> use a macro too so that I can get rid of all the if statements. After 
>> all when
>> update-cds is called like this:
>>
>>  (update-cds (where :artist "Tool") :rating 2)
>>
>> i already know that I only have to update the rating property.
>>
>> So I did this:
>>
>>  (defun make-update-expr (field value)
>>    `(progn (format t "Updating ~a to ~a" ,field ,value) (setf (getf cd 
>> ,field) ,value)))
>>
>>  (defun make-updates-list (fields)
>>    (loop while fields
>>          collecting (make-update-expr (pop fields) (pop fields))))
>>
>>  (defmacro update (&rest clauses)
>>    `(lambda (cd) ,@(make-updates-list clauses)))
>>
>>  (defun update-cds2 (selector-fn &rest update-clauses)
>>    (let ((updater-fn (update update-clauses)))
>>      (setf *db*
>>            (mapcar
>>             (lambda (row)
>>               (when (funcall selector-fn row)
>>                 (funcall updater-fn row))
>>               row)
>>             *db*))))
>>
>> but the result is rather interesting:
>>
>>  Updating (RATING 2) to NIL
>>  Updating (RATING 2) to NIL
>>
>> it seems that the update-clauses are passed to update as a list and not
>> as the individual &rest items.
> 
> 
> Put some print statements...

of type as well as argument, ie:

     (format t .... clauses (type-of clauses) (mapcar 'type-of clauses))

> in your macros to see what the input and 
> output is, and then recompile update-cds2...

...because that is when macros run, I should have reminded you.

>... to see what is going on. 
> Should be an eye-opener.

Unchanged. kt

-- 
Cells: http://common-lisp.net/project/cells/

"I'll say I'm losing my grip, and it feels terrific."
    -- Smiling husband to scowling wife, New Yorker cartoon
From: Ken Tilton
Subject: Re: Macro problem
Date: 
Message-ID: <5wATg.426$Dp.79@newsfe11.lga>
Ken Tilton wrote:
> 
> 
> Ken Tilton wrote:
> 
>>
>>
>> Stefan Arentz wrote:
>>
>>> I'm trying to do a little variation on the CD database from Practical 
>>> Common Lisp.
>>>
>>> I thought it would be a good excercise to let update-cds:
>>>
>>> (defun update-cds (selector-fn &key title artist rating (ripped nil 
>>> ripped-p))
>>>   (setf *db*
>>>         (mapcar
>>>          (lambda (row)
>>>            (when (funcall selector-fn row)
>>>              (if title (setf (getf row :title) title))
>>>              (if artist (setf (getf row :artist) artist))
>>>              (if rating (setf (getf row :rating) rating))
>>>              (if ripped-p (setf (getf row :ripped) ripped))) row)
>>>          *db*)))
>>>
>>> use a macro too so that I can get rid of all the if statements. After 
>>> all when
>>> update-cds is called like this:
>>>
>>>  (update-cds (where :artist "Tool") :rating 2)
>>>
>>> i already know that I only have to update the rating property.
>>>
>>> So I did this:
>>>
>>>  (defun make-update-expr (field value)
>>>    `(progn (format t "Updating ~a to ~a" ,field ,value) (setf (getf 
>>> cd ,field) ,value)))
>>>
>>>  (defun make-updates-list (fields)
>>>    (loop while fields
>>>          collecting (make-update-expr (pop fields) (pop fields))))
>>>
>>>  (defmacro update (&rest clauses)
>>>    `(lambda (cd) ,@(make-updates-list clauses)))
>>>
>>>  (defun update-cds2 (selector-fn &rest update-clauses)
>>>    (let ((updater-fn (update update-clauses)))
>>>      (setf *db*
>>>            (mapcar
>>>             (lambda (row)
>>>               (when (funcall selector-fn row)
>>>                 (funcall updater-fn row))
>>>               row)
>>>             *db*))))
>>>
>>> but the result is rather interesting:
>>>
>>>  Updating (RATING 2) to NIL
>>>  Updating (RATING 2) to NIL
>>>
>>> it seems that the update-clauses are passed to update as a list and not
>>> as the individual &rest items.
>>
>>
>>
>> Put some print statements...
> 
> 
> of type as well as argument, ie:
> 
>     (format t .... clauses (type-of clauses) (mapcar 'type-of clauses))
> 
>> in your macros...

...and the helper functions called at macro-expansion time...

>... to see what the input and output is, and then recompile 
>> update-cds2...
> 
> 
> ...because that is when macros run, I should have reminded you.
> 
>> ... to see what is going on. Should be an eye-opener.
> 
> 
> Unchanged. kt
> 

Next you'll be wondering how to fix the macro. Nah, this is not much of 
a place for macros. The macro (you will figure out) does not see how 
many or what keys the function takes, at expansion- or run-time.

The repetition is a problem. So you want iteration. This has the key 
ingredients:

(defun do-keys (&rest args &key one two)
   (declare (ignorable one two)) ;; kinda; bad keys still get rejected
   (loop for (k v) on args by #'cddr
         do (when v
              (print (list k (intern (symbol-name k)) v)))))

You could also do a macrolet to reduce the repetition:

    (macrolet ((upd (key))
	`(when ,key (setf (getf row
                       ,(intern (symbol-name key :keyword))
                       ,key)))
      ....
        (upd title)(upd artist)(upd rating)....

(last bit typed in as email and certainly not tested).

kt

-- 
Cells: http://common-lisp.net/project/cells/

"I'll say I'm losing my grip, and it feels terrific."
    -- Smiling husband to scowling wife, New Yorker cartoon