From: Andreas Thiele
Subject: macro style question
Date: 
Message-ID: <cfu5l4$utn$03$1@news.t-online.com>
Hi,

I am writing a lisp ODBC package for database access. At the moment using my
package you can write the following code to retrieve a range of rows:

(in-package :user)

(defun test-std ()
  (let (cm rs (from 2105) (to 2110))
    (setf cm (odbc:new-command "select * from test1 where id>=? and id<=?"))
    (setf (odbc:parameter-value cm 0) from) ;; bind first
    (setf (odbc:parameter-value cm 1) to) ;; and second parameter
    (setf rs (odbc:new-recordset cm))
    (when (zerop (odbc:sql-code))
      (loop
        (when (odbc:rs-eof rs) (return))
        (print (mapcar #'(lambda (x) (getf x :value)) (odbc:rs-fields rs)))
        (odbc:rs-movenext rs)))
    (odbc:rs-flush rs) ;; close recordset and free foreign objects
    (odbc:cmd-flush cm)) ;; dito
  "OK")

Now I am considering simplifications, so one can write instead:

(defun test-adv ()
  (let ((from 2105) (to 2110))
    (odbc::with-cmd (cm "select * from test1 where id>=? and id<=?" from to)
      (odbc::with-rs (rs cm)
        (when (zerop (odbc:sql-code))
          (loop
            (when (odbc:rs-eof rs) (return))
            (print (mapcar #'(lambda (x) (getf x :value)) (odbc:rs-fields
rs)))
            (odbc:rs-movenext rs))))))
  "OK")

I accomplish this by two small very similar macros:

(in-package :odbc)

;; automatic house keeping
(defmacro with-cmd (cmd-spec &body body)
  `(let (,(car cmd-spec))
     (unwind-protect
       (progn
         (setf ,(car cmd-spec) (new-command ,(second cmd-spec)))
         (let ((i -1))
           (dolist (parm (list ,@(cddr cmd-spec))) ;; automatically bind an
arbitrary number of parms
             (setf (parameter-value ,(car cmd-spec) (incf i)) parm)))
         ,@body)
       (cmd-flush ,(car cmd-spec)))))

(defmacro with-rs (rs-spec &body body)
  `(let (,(car rs-spec))
     (unwind-protect
       (progn
         (setf ,(car rs-spec) (new-recordset ,(second rs-spec)))
         ,@body)
       (rs-flush ,(car rs-spec)))))

The code works. I am just uncertain if the style is OK. Did I miss
something? Do you know any improvements when considering the two macros?

Thanks Andreas

From: Barry Margolin
Subject: Re: macro style question
Date: 
Message-ID: <barmar-85003E.00401318082004@comcast.dca.giganews.com>
In article <···············@news.t-online.com>,
 "Andreas Thiele" <··········@nospam.com> wrote:

> I accomplish this by two small very similar macros:
> 
> (in-package :odbc)
> 
> ;; automatic house keeping
> (defmacro with-cmd (cmd-spec &body body)
>   `(let (,(car cmd-spec))
>      (unwind-protect
>        (progn
>          (setf ,(car cmd-spec) (new-command ,(second cmd-spec)))
>          (let ((i -1))
>            (dolist (parm (list ,@(cddr cmd-spec))) ;; automatically bind an
> arbitrary number of parms
>              (setf (parameter-value ,(car cmd-spec) (incf i)) parm)))
>          ,@body)
>        (cmd-flush ,(car cmd-spec)))))
> 
> (defmacro with-rs (rs-spec &body body)
>   `(let (,(car rs-spec))
>      (unwind-protect
>        (progn
>          (setf ,(car rs-spec) (new-recordset ,(second rs-spec)))
>          ,@body)
>        (rs-flush ,(car rs-spec)))))
> 
> The code works. I am just uncertain if the style is OK. Did I miss
> something? Do you know any improvements when considering the two macros?

Use destructuring rather than things like (car cmd-spec), (cddr 
cmd-spec), etc.:

(defmacro with-cmd ((var command &rest vars) &body body)
  `(let (,var)
     (unwind-protect
         (progn
           (setf ,var (new-command ,command))
           (loop for i upfrom 0
                 for item in (list ,@vars)
              do (setf (parameter-value ,var i) item))
           ,@body)
      (cmd-flush ,var))))

I also used LOOP to iterate through the list at the same time as 
iterating I automatically.

-- 
Barry Margolin, ······@alum.mit.edu
Arlington, MA
*** PLEASE post questions in newsgroups, not directly to me ***
From: Andreas Thiele
Subject: Re: macro style question
Date: 
Message-ID: <cfv1uh$ns$06$1@news.t-online.com>
"Barry Margolin" <······@alum.mit.edu> schrieb im Newsbeitrag
·································@comcast.dca.giganews.com...
> In article <···············@news.t-online.com>,
>  "Andreas Thiele" <··········@nospam.com> wrote:
>
> > I accomplish this by two small very similar macros:
> >
> > (in-package :odbc)
> >
> > ;; automatic house keeping
> > (defmacro with-cmd (cmd-spec &body body)
> >   `(let (,(car cmd-spec))
> >      (unwind-protect
> >        (progn
> >          (setf ,(car cmd-spec) (new-command ,(second cmd-spec)))
> >          (let ((i -1))
> >            (dolist (parm (list ,@(cddr cmd-spec))) ;; automatically bind
an
> > arbitrary number of parms
> >              (setf (parameter-value ,(car cmd-spec) (incf i)) parm)))
> >          ,@body)
> >        (cmd-flush ,(car cmd-spec)))))
> >
> > (defmacro with-rs (rs-spec &body body)
> >   `(let (,(car rs-spec))
> >      (unwind-protect
> >        (progn
> >          (setf ,(car rs-spec) (new-recordset ,(second rs-spec)))
> >          ,@body)
> >        (rs-flush ,(car rs-spec)))))
> >
> > The code works. I am just uncertain if the style is OK. Did I miss
> > something? Do you know any improvements when considering the two macros?
>
> Use destructuring rather than things like (car cmd-spec), (cddr
> cmd-spec), etc.:
>
> (defmacro with-cmd ((var command &rest vars) &body body)
>   `(let (,var)
>      (unwind-protect
>          (progn
>            (setf ,var (new-command ,command))
>            (loop for i upfrom 0
>                  for item in (list ,@vars)
>               do (setf (parameter-value ,var i) item))
>            ,@body)
>       (cmd-flush ,var))))
>
> I also used LOOP to iterate through the list at the same time as
> iterating I automatically.
>
> --
> Barry Margolin, ······@alum.mit.edu
> Arlington, MA
> *** PLEASE post questions in newsgroups, not directly to me ***

Thank you! Really improved readability (Ooops).

Perhaps I should rename the macro to with-command. I noticed that I use two
different naming conventions. While I have something like 'new-recordset' I
have 'rs-movenext' because I find it unpleasent to always have to write
'recordset-movenext'. Nevertheless would it be better practice to rename
'rs-movenext' to 'recordset-movenext' etc. or is such a
'short-hand-notation' acceptable?

Andreas