Hi!
I'm very new to lisp and thought it would be good to practice a little.
I stumbled upon this page:
http://www.cs.northwestern.edu/academics/courses/325/exercises/lisp-exs.html
and I have a problem right with the second exercise, which is this:
Lisp #2: KEY-IF (Wilensky 13.2)
Define the macro key-if to have the form
(KEY-IF test
:THEN exp1 exp2 ...
:ELSE exp3 exp4 ...)
This does about the same thing as:
(COND (test exp exp ...)
(T else-exp else-exp ...))
Almost everything is optional in key-if except the test. Here are some
legal forms and their results:
> (key-if (> 3 1) :then 'ok)
OK
> (key-if (< 5 3) :else 'ok)
OK
> (key-if (> 3 1) :else 'oops)
NIL
> (key-if (> 3 1) :then)
NIL
> (key-if (> 3 1) :else 'oops :then 'ok)
OK
> (key-if (> 3 1) :else 'oops :then (print 'hi) 'ok)
HI
OK
I've came up with a solution, which is this:
(defmacro key-if (test &rest exps &aux (then ()) (else ()))
(if (not (keywordp (first exps))) (error "the first expression must be
a keyword"))
(let ((thenp t))
(dolist (e exps)
(if (keywordp e)
(cond ((eql e :then) (setf thenp t))
((eql e :else) (setf thenp nil))
(t (error "the keyword ~a is unknown" e))
)
(if thenp (setf then (append then `(,e))) (setf else (append else
`(,e))))
)
)
)
(if (eql then nil) (setf then '(nil)))
(if (eql else nil) (setf else '(nil)))
`(cond (,test ,@then)
(t ,@else)
)
)
I'm not satisfied with my solution, it seems to do the work but there's
to much plumbing in it.
How can it be done better?
Peter
Peter Nagy <········@elte.hu> wrote:
+---------------
| I've came up with a solution, which is this: ...[snipped]...
| I'm not satisfied with my solution, it seems to do the work
| but there's too much plumbing in it. How can it be done better?
+---------------
First, please learn to write with standard Lisp indentation and
closing parens on the same line. This is *very* important, since
without it your code is nearly unreadable to Lispers. Next, forget
about &AUX variables; they buy you little or nothing compared to
LET or LET*. Less importantly, though still contributing to readability,
use WHEN & UNLESS (as appropriate) to eliminate single-branch IFs.
Finally, where possible, ERROR should report the erring form [with
"~s", not "~a", to avoid ambiguity], not just *that* something went
wrong. Put that all together, and your code becomes much more readable:
(defmacro key-if (test &rest exps)
(let (then else)
(when (not (keywordp (first exps)))
(error "The first expression is not a keyword: ~s" (first exps)))
(let ((thenp t))
(dolist (e exps)
(if (keywordp e)
(cond
((eql e :then) (setf thenp t))
((eql e :else) (setf thenp nil))
(t (error "Unknown keyword: ~s" e)))
(if thenp
(setf then (append then `(,e)))
(setf else (append else `(,e)))))))
(unless then
(setf then '(nil)))
(unless else
(setf else '(nil)))
`(cond (,test ,@then)
(t ,@else))))
O.k., now that I see what it's doing, there's one obvious
simplification: emit an IF instead of a COND at the end, which
means you'll need initial PROGNs for both cases, but that means
that instead of all that quasiquoting and APPENDing, you can just
PUSH the subforms onto the THEN & ELSE variables and then emit
them reversed at the end. [You can also simplify the IF/COND
in the loop a bit; and you don't need to initialize THENP since
it's *always* set before being used.] Results:
(defmacro key-if (test &rest exps)
(let (then else thenp)
(when (not (keywordp (first exps)))
(error "The first expression is not a keyword: ~s" (first exps)))
(dolist (e exps)
(cond
((eql e :then) (setf thenp t))
((eql e :else) (setf thenp nil))
((keywordp e) (error "Unknown keyword: ~s" e))
(t (if thenp
(push e then)
(push e else)))))
`(if ,test
(progn ,@(reverse then))
(progn ,@(reverse else)))))
Now it becomes clear that there's nothing preventing *multiple*
:THEN or :ELSE keywords from being used, with the forms following
each being accumulated into the corresponding IF branch, in order:
> (macroexpand '(key-if (> x 3)
:else (foo)
then (this) (that)
:else (bar)
:then (the-other)))
(IF (> X 3)
(PROGN
(THIS)
(THAT)
(THE-OTHER))
(PROGN
(FOO)
(BAR)))
T
>
Is that what you would expect, given the problem statement?
-Rob
-----
Rob Warnock <····@rpw3.org>
627 26th Avenue <URL:http://rpw3.org/>
San Mateo, CA 94403 (650)572-2607
····@rpw3.org (Rob Warnock) writes:
> Peter Nagy <········@elte.hu> wrote:
> +---------------
> | I've came up with a solution, which is this: ...[snipped]...
> | I'm not satisfied with my solution, it seems to do the work
> | but there's too much plumbing in it. How can it be done better?
> +---------------
>
> First, please learn to write with standard Lisp indentation and
> closing parens on the same line. This is *very* important, since
> without it your code is nearly unreadable to Lispers. Next, forget
> about &AUX variables; they buy you little or nothing compared to
> LET or LET*. Less importantly, though still contributing to readability,
> use WHEN & UNLESS (as appropriate) to eliminate single-branch IFs.
> Finally, where possible, ERROR should report the erring form [with
> "~s", not "~a", to avoid ambiguity], not just *that* something went
> wrong. Put that all together, and your code becomes much more readable:
Unfortunately, you used TAB, so the code became much less readable:
> (defmacro key-if (test &rest exps)
> (let (then else)
> (when (not (keywordp (first exps)))
> (error "The first expression is not a keyword: ~s" (first exps)))
> (let ((thenp t))
> (dolist (e exps)
> (if (keywordp e)
> (cond
> ((eql e :then) (setf thenp t))
> ((eql e :else) (setf thenp nil))
> (t (error "Unknown keyword: ~s" e)))
> (if thenp
> (setf then (append then `(,e)))
> (setf else (append else `(,e)))))))
> (unless then
> (setf then '(nil)))
> (unless else
> (setf else '(nil)))
> `(cond (,test ,@then)
> (t ,@else))))
>
> O.k., now that I see what it's doing, there's one obvious
> simplification: emit an IF instead of a COND at the end, which
> means you'll need initial PROGNs for both cases, but that means
> that instead of all that quasiquoting and APPENDing, you can just
> PUSH the subforms onto the THEN & ELSE variables and then emit
> them reversed at the end. [You can also simplify the IF/COND
> in the loop a bit; and you don't need to initialize THENP since
> it's *always* set before being used.] Results:
>
> (defmacro key-if (test &rest exps)
> (let (then else thenp)
> (when (not (keywordp (first exps)))
> (error "The first expression is not a keyword: ~s" (first exps)))
> (dolist (e exps)
> (cond
> ((eql e :then) (setf thenp t))
> ((eql e :else) (setf thenp nil))
> ((keywordp e) (error "Unknown keyword: ~s" e))
> (t (if thenp
> (push e then)
> (push e else)))))
> `(if ,test
> (progn ,@(reverse then))
> (progn ,@(reverse else)))))
Why do you forbid: (key-if predicate-p :then :enabled :else :disabled) ?
> Now it becomes clear that there's nothing preventing *multiple*
> :THEN or :ELSE keywords from being used, with the forms following
> each being accumulated into the corresponding IF branch, in order:
Why should there?
> > (macroexpand '(key-if (> x 3)
> :else (foo)
> then (this) (that)
> :else (bar)
> :then (the-other)))
> (IF (> X 3)
> (PROGN
> (THIS)
> (THAT)
> (THE-OTHER))
> (PROGN
> (FOO)
> (BAR)))
> T
> >
>
> Is that what you would expect, given the problem statement?
(defmacro key-if (test &body body)
(loop
:with statements = (make-array 3 :initial-element '())
:with seen = 0
:for item :in body
:do (case item
((:then) (setf seen 1))
((:else) (setf seen 2))
(otherwise (push item (aref statements seen))))
:finally
(when (not (null (aref statements 0)))
(warn "Eliminating dead code: ~S" (nreverse (aref statements 0))))
(return `(cond (,test ,@(nreverse (aref statements 1)))
(t ,@(nreverse (aref statements 2)))))))
There's no need to generate a IF, because the underlying Common Lisp
implementation can do it for you:
[127]> (macroexpand-1 '(key-if predicate-p :then :enabled :else :disabled))
(COND (PREDICATE-P :ENABLED) (T :DISABLED)) ;
T
[128]> (macroexpand '(key-if predicate-p :then :enabled :else :disabled))
(IF PREDICATE-P :ENABLED :DISABLED) ;
T
[131]> (macroexpand-1 '(key-if predicate-p (print 'hi!)
:then (print 'yes) (terpri)
:else (print 'no) (terpri)
:then :enabled :else :disabled))
WARNING: Eliminating dead code: ((PRINT 'HI!))
(COND (PREDICATE-P (PRINT 'YES) (TERPRI) :ENABLED)
(T (PRINT 'NO) (TERPRI) :DISABLED)) ;
T
--
__Pascal Bourguignon__ http://www.informatimago.com/
"Our users will know fear and cower before our software! Ship it!
Ship it and let them flee like the dogs they are!"
Pascal Bourguignon <···@informatimago.com> wrote:
+---------------
| ····@rpw3.org (Rob Warnock) writes:
| > Peter Nagy <········@elte.hu> wrote:
| > +---------------
| > | I've came up with a solution, which is this: ...[snipped]...
| > | I'm not satisfied with my solution, it seems to do the work
| > | but there's too much plumbing in it. How can it be done better?
| > +---------------
| >
...
| > Put that all together, and your code becomes much more readable:
|
| Unfortunately, you used TAB, so the code became much less readable:
+---------------
Hmmm... Sorry 'bout that. Not sure where the TABs came in -- I used
only spaces when indenting.
+---------------
| Why do you forbid: (key-if predicate-p :then :enabled :else :disabled) ?
+---------------
"I" didn't, Nagy's original code did. Not having access to the
problem statement he was solving, I didn't know if his specific
exclusion of other keywords was in the problem or not.
+---------------
| > Now it becomes clear that there's nothing preventing *multiple*
| > :THEN or :ELSE keywords from being used, with the forms following
| > each being accumulated into the corresponding IF branch, in order:
...
| > Is that what you would expect, given the problem statement?
|
| Why should there?
+---------------
See above about not having read the original problem statement
in the book. One might want to, in some case; one might not.
+---------------
| (defmacro key-if (test &body body)
| (loop ... ))
+---------------
Well, since he seemed to be a relative newbie, I decided *not*
to introduce LOOP myself. Had I used LOOP, I probably would have
done it much as you did.
+---------------
| There's no need to generate a IF, because the underlying Common Lisp
| implementation can do it for you:
+---------------
Yes, well, historically Lisp implementations have varied in
which of IF/COND is "primitive".
-Rob
-----
Rob Warnock <····@rpw3.org>
627 26th Avenue <URL:http://rpw3.org/>
San Mateo, CA 94403 (650)572-2607
····@rpw3.org (Rob Warnock) writes:
> First, please learn to write with standard Lisp indentation and
> closing parens on the same line. This is *very* important, since
> without it your code is nearly unreadable to Lispers.
Aren't parens supposed to be invisible? :-)
--
__("< Marcin Kowalczyk
\__/ ······@knm.org.pl
^^ http://qrnik.knm.org.pl/~qrczak/
Marcin 'Qrczak' Kowalczyk <······@knm.org.pl> wrote:
+---------------
| ····@rpw3.org (Rob Warnock) writes:
| > First, please learn to write with standard Lisp indentation and
| > closing parens on the same line. This is *very* important, since
| > without it your code is nearly unreadable to Lispers.
|
| Aren't parens supposed to be invisible? :-)
+---------------
Yes, *iff* the code is indented correctly
[for a wide variety of values of "correctly"]...
-Rob
-----
Rob Warnock <····@rpw3.org>
627 26th Avenue <URL:http://rpw3.org/>
San Mateo, CA 94403 (650)572-2607
On 9511 day of my life Marcin Kowalczyk wrote:
>> First, please learn to write with standard Lisp indentation and
>> closing parens on the same line. This is *very* important, since
>> without it your code is nearly unreadable to Lispers.
>
> Aren't parens supposed to be invisible? :-)
They are invisible when at proper place. When they aren't, they hurt
our eyes.
--
Ivan Boldyrev
Many are called, few volunteer.