Recursive function that does not terminate!

[pupsik]

The following recursive function does not
terminate if I exexute it in my REPL.
What is wrong?
(This example is from the official Clojure-website).

(defn my-zipmap [keys vals]
(loop [my-map {}
my-keys (seq keys)
my-vals (seq vals)]
(if (and my-keys my-vals)
(recur (assoc my-map (first my-keys) (first my-vals))
(rest my-keys)
(rest my-vals))
my-map)))

(my-zipmap [:a :b :c] [1 2 3])

[Cosmin Stejerean]

On Tue, Jun 23, 2009 at 5:22 PM, pupsik <an_ni...@yahoo.de> wrote:

(defn my-zipmap [keys vals]
 (loop [my-map {}
        my-keys (seq keys)
        my-vals (seq vals)]
   (if (and my-keys my-vals)
     (recur (assoc my-map (first my-keys) (first my-vals))
            (rest my-keys)
            (rest my-vals))
     my-map)))

Well, it seems like the for loop never terminates. (rest ()) => () and as far as I can tell an empty sequence is logically true, not false. (if (rest ()) 1 2) => 1 so your if will never hit the else clause.

Here's a version that will do what you expect.

(defn my-zipmap [keys vals]

 (loop [my-map {}

        my-keys (seq keys)

        my-vals (seq vals)]

   (if-not (or (empty? my-keys) (empty? my-vals))

     (recur (assoc my-map (first my-keys) (first my-vals))

            (rest my-keys)

            (rest my-vals))

     my-map)))

user=> (my-zipmap [:a :b :c] [1 2 3])                            

{:c 3, :b 2, :a 1}

--
Cosmin Stejerean
http://offbytwo.com

[Stephen C. Gilardi]

The example needs to be updated for the lazier seqs that were
implemented before the Clojure 1.0 release. As part of that change, we
lost "nil punning". "rest" no longer returns nil if there are no more
items. A new function "next" does though:

user=> (source zipmap)
(defn zipmap
"Returns a map with the keys mapped to the corresponding vals."
[keys vals]
(loop [map {}
ks (seq keys)
vs (seq vals)]
(if (and ks vs)
(recur (assoc map (first ks) (first vs))
(next ks)
(next vs))
map)))
nil
user=> (doc rest)
-------------------------
clojure.core/rest
([coll])
Returns a possibly empty seq of the items after the first. Calls
seq on its
argument.
nil
user=> (doc next)
-------------------------
clojure.core/next
([coll])
Returns a seq of the items after the first. Calls seq on its
argument. If there are no more items, returns nil.
nil

This is the example that needs updating at clojure.org:

http://clojure.org/functional_programming#toc7

--Steve

[Rich Hickey]

Yes, this (Steve's) version, using next and testing directly with (and
ks vs) is the idiomatic way when you are not in turn producing a lazy
seq.

(if-not (or (empty? ks) (empty? vs))

is to be avoided. (No offense Cosmin :)

Rich

[Cosmin Stejerean]

None taken. As I was writing that code I was wondering why it had to look so ugly. I completely forgot about next.