HiveBrain v1.2.0
Get Started
← Back to all entries
principleMinor

Recursion vs. iteration in Lisp macro

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
iterationlispmacrorecursion

Problem

I've been programming Clojure for a little while and recently started learning Common Lisp. One of my favorite things about Clojure is the threading operator ->, which greatly simplifies long chains of nested function calls. Naturally I wanted to have this in Common Lisp.

I found an implementation here:

(defmacro -> (x &rest args)
  (destructuring-bind (form &rest more)
      args
    (cond
      (more `(-> (-> ,x ,form) ,@more))
      ((and (consp form)
            (or (eq (car form) 'lambda)
                (eq (car form) 'function)))
       `(funcall ,form ,x))
      ((consp form) `(,(car form) ,x ,@(cdr form)))
      (form `(,form ,x))
      (t x))))


This uses a recursive macro expansion; I've read that it's better to use iteration over recursion in CL when you can, so I wrote my own version:

(defmacro -> (x &rest forms)
  (labels ((expand-form (x form)
             (if (consp form)
                 (if (or (eq (car form) 'lambda)
                         (eq (car form) 'function))
                     `(funcall ,form ,x)
                     `(,(car form) ,x ,@(cdr form)))
                 `(,form ,x))))
    (do ((forms forms (cdr forms))
         (x x (expand-form x (car forms))))
        ((not forms) x))))


I'm relatively new to Lisp so I don't know how to judge which way is better. Any comments, suggestions?

EDIT: I knew something was fishy! You don't have to use any looping constructs at all, a plain old reduce will cut it:

(defmacro -> (x &rest forms)
  (flet ((expand-form (x form)
           (cond
             ((atom form)
              `(,form ,x))
             ((member (car form) '(lambda function))
              `(funcall ,form ,x))
             (t
              `(,(car form) ,x ,@(cdr form))))))
    (reduce #'expand-form forms :initial-value x)))

Solution

I think the first version is slightly more elegant and looks okay to me.

In the second version, I would replace LABELS with FLET. LABELS would indicate some kind of recursion.

Generally I would also think if the nested expansion is something I would want, vs. an expansion into a LET* form.

I would also replace CAR and CDR with FIRST and REST.

The DESTRUCTURING-BIND is also not necessary:

(defmacro -> (x form &rest more)
  (cond (more
         `(-> (-> ,x ,form) ,@more))
        ((and (consp form)
              (member (first form) '(lambda function)))
         `(funcall ,form ,x))
        ((consp form)
         `(,(first form) ,x ,@(rest form)))
        (form
         `(,form ,x))
        (t x)))

Code Snippets

(defmacro -> (x form &rest more)
  (cond (more
         `(-> (-> ,x ,form) ,@more))
        ((and (consp form)
              (member (first form) '(lambda function)))
         `(funcall ,form ,x))
        ((consp form)
         `(,(first form) ,x ,@(rest form)))
        (form
         `(,form ,x))
        (t x)))

Context

StackExchange Code Review Q#77150, answer score: 2

Revisions (0)

No revisions yet.