clojure.zip/seq-zip returns spurious nils during traversal

Question

Problem reported by Lee Spector on the mailing list:

https://groups.google.com/d/msg/clojure/8TL7IGmE7N0/u1xfgTOLDRgJ

Here's a quote from Lee's post describing the problem:

`
Here's an illustration, stepping through '(() 0) with next and printing the node at each step:

(loop [z (zip/seq-zip '(() 0))]
(if (zip/end? z)

:done 
(do (println (zip/node z)) 
  (recur (zip/next z))))) 

That produces:

(() 0)
()
nil
0
:done

I don't expect the nil to be there.
`

The underlying cause is that {{seq-zip}} passes {{identity}} as the {{children}} argument to {{zipper}}. Applied to {{()}}, this returns {{()}}, which is truthy, leading {{zipper}} to descend into a non-existent subtree.

One natural solution would be to use {{seq}} in place of {{identity}}:

`
(defn seq-zip [root]
(zipper seq?

      seq  ;; changed
      (fn [node children] (with-meta children (meta node)))
      root))

`

With this change, no {{nil}} is produced in the example above. Patch with this change forthcoming.

Answer 1

Comment made by: michalmarczyk

Note that the docstring of {{clojure.zip/zipper}} asks that the {{children}} argument return a seq of children. The rest of {{clojure.zip}}, however, expects {{nil}} to be returned when there are no children, as evidenced by this problem.

One could argue that this behaviour of the rest of {{clojure.zip}} should be fixed, but I think it makes sense and is convenient. Perhaps the docstring should be adjusted, though.

Answer 2

_Comment made by: alexmiller_

Michał, can I ask why you assigned this to yourself - was there something you planned to add?

Answer 3

_Comment made by: michalmarczyk_

Hey Alex, I was going to attach a separate patch with a proposal for a docstring adjustment along the lines suggested above (will do that tonight). No change to the code, though, and I guess not worth assigning the ticket – sorry about the unnecessary ping.

Answer 4

Comment made by: alexmiller

No worries, just wanted to know if something was still pending - I will wait to prescreen it.

Answer 5

Reference: https://clojure.atlassian.net/browse/CLJ-1317 (reported by michalmarczyk)