Peter Nagy  <arrostia@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@rpw3.org>
627 26th Avenue			<URL:http://rpw3.org/>
San Mateo, CA 94403		(650)572-2607