[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: start-picture/get-picture question

 > From: Rob Browning <osiris@cs.utexas.edu>
 > (defmacro append-to-auto-redraw-window (win &rest items)
 >   `(with-focused-view ,win
 >      (start-picture ,win)
 >      (if (contents ,win) (draw-picture ,win (contents ,win)))
 >      ,@items
 >      (if (contents ,win) (kill-picture (contents ,win)))
 >      (setf (contents ,win) (get-picture ,win))))

I second Steve Strassmann's comments about macro abuse.  I'd like to
add a couple of points about good macro style.

First, it is important to avoid evaluating argument forms more than
once.  In the above example, WIN is evaluated nine times.  If the
first argument is a form that does some computation then this can get
expensive.  For example:

   (append-to-auto-redraw-window (find-window ...)
     <body>)

Worse, if it's a form that has side effects those side effects will
happen multiple times.

Second, you should take care to preserve the standard Common Lisp
semantics of evaluating arguments from left to right.  You may not
agree that specifying this was a `good thing' for Common Lisp to have
done, but it is the defined semantics.  Maintaining left-to-right
evaluation will save other people using your code (and maybe even you,
yourself) grief later on.

Third, any macro that takes a body (a sequence of forms to be executed
as if they were enclosed in a progn), as this macro does, should
return the value of the last form in the body.  This makes it easier
for callers to return values based on some context established in the
body.  It also fits in with all the Common Lisp forms that accept
bodys.

Finally, macros are often used to establish an environment in which
some forms will be executed.  That's what this macro does. Such macros
should take care that if the body is exited abnormally (either because
of an error or because of a non-local exit) the environment is left in
a resonable state.  For example, in this macro unless the GET-PICTURE
is executed many unintended quickdraw operations will be recorded.
Worse, the next call to start-picture will cause an error because you
can only record one picture at a time.  The Common Lisp special form
UNWIND-PROTECT is designed for exactly this situation.

A better implementation would be as follows.  It's up to you, as the
macro writer, to decide what the appropriate cleanup activities are if
an abnormal exit occurs.  This implementation just saves the possibly
incomplete picture.

   (defmacro append-to-auto-redraw-window (win &rest items)
     (let ((the-window (gensym)))
       `(let ((,the-window ,win))
          (with-focused-view ,the-window
            (unwind-protect
                (progn
                  (start-picture ,the-window)
                  (if (contents ,the-window)
                      (draw-picture ,the-window (contents ,the-window)))
                  ,@items)
              (if (contents ,the-window)
                  (kill-picture (contents ,the-window)))
              (setf (contents ,the-window) (get-picture ,the-window))))))


Kevin Gallagher