1

I was reading the book On Lisp by Paul Graham. In Chapter 4, Utility Functions, he gives examples of small functions that operate on lists, which would be helpful while writing a larger program.

One of them is flatten. Given a nested list at any arbitrary level as argument, flatten will remove all the nested elements and put them on the top level.

Below is my attempt at implementing flatten:

(defun flatten (lst)
  (labels ((rflatten (lst1 acc)
                     (dolist (el lst1)
                       (if (listp el)
                         (rflatten el acc)
                         (push el acc)))
                     acc))
    (reverse (rflatten lst nil))))

But the above function does not flatten lists properly.

; returns (1) instead of (1 2)
(print (flatten '(1 (2))))
(1)

Calling the function with (1 (2)) returns (1) instead of (1 2).

I cannot find what's wrong with my implementation of flatten. Is it the way I am using labels? Or is it the the way I am using the dolist macro? The dolist macro always returns nil. But that should not matter as I am using an accumulator acc to store the flattened list.

Will Ness
  • 70,110
  • 9
  • 98
  • 181
ardsrk
  • 2,407
  • 2
  • 21
  • 33
  • 4
    There are [lots of questions about flatten](http://stackoverflow.com/search?q=%5Blisp%5D+flatten+is%3Aquestion); this *might* be a duplicate of one of them. If not, those still might be helpful. – Joshua Taylor Sep 16 '14 at 10:54
  • 1
    @JoshuaTaylor crazy thing about that is that most [tag:lisp] question with `flatten` in them are either Scheme questions or request for similar to but not exactly like `flatten` questions. I know there is a good answer by Will amongst them but it's easier to just answer than to find it I guess. Besides he needed help with his implementation and I didn't comment on how he actually did it since in CL standards it looked ok for me. (note to self: bookmark good answers) – Sylwester Sep 16 '14 at 20:01
  • 2
    As I said, I didn't find a duplicate either in a few moments of searching, but I expect someone who finds *this* question about flattening a list may also find *those* questions about flattening lists useful as well. – Joshua Taylor Sep 16 '14 at 20:02
  • In the Alexandria library: `flatten` https://common-lisp.net/project/alexandria/draft/alexandria.html#Conses "Traverses the tree in order, collecting non-null leaves into a list.". – Ehvince Oct 09 '17 at 16:43

4 Answers4

3

push changes the symbol binding in scope. Thus the recursion (rflatten el acc) has it's own acc which is the result there but you don't do anything with the returned result and it doesn't alter the callee acc.

Perhaps a (setf acc (rflatten el acc)) would fix that:

(defun flatten (lst)
  (labels ((rflatten (lst1 acc)
             (dolist (el lst1)
               (if (listp el)
                   (setf acc (rflatten el acc))
                   (push el acc)))
             acc))
    (reverse (rflatten lst nil))))
Sylwester
  • 47,942
  • 4
  • 47
  • 79
  • However, I find it odd that a function call in common-lisp creates a scope on its arguments. Ruby just hands the object's reference passed as an argument to the called function. – ardsrk Sep 16 '14 at 18:44
  • @ardsrk `push` is a macro (do `(macroexpand '(push a b))`. In ruby it would be like you had a variable `acc` which is local (argument like in our case) and you do `acc = [e1] + acc`. Thus if you did a copy like `tmp = acc` before that you'd notice `acc` points at something new but `tmp` points to the old value(object) it had. We didn't change the object but the reference. I don't think Ruby has much different scope rules than CL except it has more scopes than CL. There is a [functional Ruby guide](https://code.google.com/p/tokland/wiki/RubyFunctionalProgramming) and CL would be more like that. – Sylwester Sep 16 '14 at 19:41
3

You're actually very close. As Sylwester mentions, the issue is that (push el acc) only modifies the local binding of el (of which there's a new one for each call to rflatten. As Rainer mentions, it's not really an accumulator in the traditional sense, so I'm going not going to call it acc, but result. Since you're already defining a local function, there's no reason not to define result in a wider scope:

(defun flatten (lst)
  (let ((result '()))
    (labels ((rflatten (lst1)
               (dolist (el lst1)
                 (if (listp el)
                   (rflatten el)
                   (push el result)))))
      (rflatten lst)
      (nreverse result))))

There are actually a few ways to clean this up, too. The first is a matter of style and taste, but I'd use an &aux variable to bind result, so

(defun flatten (lst &aux (result '()))
  ...)

The next is that dolist can take a third argument, a form to evaluate as for the return value. This is often used in a "push to create a list, then reverse for the return value" idiom, e.g.,

(let ((result '()))
  (dolist (x list (nreverse result))
    ...
    (push ... result)))

You don't want to reverse after every dolist, but you can still return result from the dolist, and thus from rflatten. Then you can simply call nreverse with the result of rflatten:

(defun flatten (lst &aux (result '()))
  (labels ((rflatten (lst1)
             (dolist (el lst1 result)
               (if (listp el)
                 (rflatten el)
                 (push el result)))))
      (nreverse (rflatten lst))))
Joshua Taylor
  • 84,998
  • 9
  • 154
  • 353
  • Didn't know about &aux. Thanks. – ardsrk Sep 16 '14 at 11:26
  • 'acc' is not really an accumulator in the original sense (an accumulator in a TCO style function), it basically is the result value. Thus I would name it flattened-list or result or... – Rainer Joswig Sep 16 '14 at 13:15
  • @JoshuaTaylor: so how would a fully tail recursive version look like - using an accumulator? ;-) – Rainer Joswig Sep 16 '14 at 13:28
  • Well, since when you encounter a list like `((...) ...)`, you still have to flatten the car and the cdr of the list, no matter which you do first there will still be more to do afterward. A "mostly tail" recursive version" would be http://pastebin.com/LArtrpBb. Doing a "fully tail recursive with accumulator" version of this would almost be continuation passing style. – Joshua Taylor Sep 16 '14 at 13:44
  • 2
    @RainerJoswig How about [this](https://gist.github.com/westerp/cbc1b0434cc2b52e9b10)? Anyway you have no guarantee for TCO with CL so a `loop` version would be better. I wonder how that would look like :-) – Sylwester Sep 16 '14 at 14:57
  • 1
    @ JoshuaTaylor I've added an answer with a code with a primitive LOOP, following comments and starting from a code from above comment by @Sylwester. no CPS, lots of SETQ! – Will Ness Feb 21 '18 at 03:46
2

A non-recursive code which builds the result by conses, following comments and starting from a code by user:Sylwester:

(defun flatten (lst &optional back acc)
  (loop
     (cond 
        ((consp lst) (psetq lst (cdr lst)              ; parallel assignment
                           back (cons (car lst) back)))
        (back
                  (if (consp (car back))  
                    (psetq lst (cdar back)
                          back (cons (caar back) (cdr back)))
                    (psetq acc (if (car back) (cons (car back) acc) acc)
                          back (cdr back))))
        (t     
               (return acc)))))                        ; the result

It's not pretty, but it seems to work. Parallel assignment PSETQ is used to simulate tail-recursive call frame update without worrying about precise sequencing.

Implements the same process as the one encoded nicely by

(defun flatten2 (l z)
    (cond
        ((endp l) z)
        ((listp (car l)) (flatten2 (car l) (flatten2 (cdr l) z)))
        ((atom (car l)) (cons (car l) (flatten2 (cdr l) z)))))

(defun flatten (l)
   (flatten2 l nil))

with implicit stack operations explicated as list structure manipulations among the variables.

Will Ness
  • 70,110
  • 9
  • 98
  • 181
1

I discovered a solution which does not use helper functions or variable assignment, and constructs the list in a forward manner, which I think is easier to understand.

(defun flatten (lst &aux (re '()))
  (cond
    ((null lst) '())
    ((listp (car lst))
     (append (flatten (car lst))
             (append (flatten (cdr lst))
                     re)))
    (t (cons (car lst)
             (append (flatten (cdr lst)) re)))))

And we can easily adapt it to control the depth of the flattening!

(defun flatten* (lst depth &aux (re '()))
  (cond
    ((null lst) '())
    ((listp (car lst))
     (append (cond
               ((= 0 depth)             ; flatten none
                (list (car lst)))
               ((< 0 depth)             ; flatten down
                (flatten* (car lst) (- depth 1)))
               ((= -1 depth)            ; flatten all
                (flatten* (car lst) depth))
               ((< depth -1)            ; flatten up
                (list (flatten* (car lst) (+ depth 1)))))
             (append (flatten* (cdr lst) depth)
                     re)))
    (t (cons (car lst)
             (append (flatten* (cdr lst) depth) re)))))