Failing to disassociate vector items? Stuck in a loop

2
votes

I'm writing a loop which creates a map from string data in a vector. In each iteration I take the last item in the vector, construct a key and a value from it, associate those with the map and then pop the item just logged to the map off of the vector. As long as the vector still has items in it, this process should keep repeating until the vector is empty and the map is full.

The function reads as follows:

(defn map-builder
  "Transforms the vector of waterfalls into a map of waterfalls."
  [input]
  (loop [waterfall-db {}
        vectorized-db input]
    (let [key-str (last vectorized-db)]
    key (->> key-str
            (re-seq #"[0-9]+")
             keyword)
    val (subs key-str (+ 2 (.indexOf key-str ":"))))
    (assoc waterfall-db key val)
    (pop vectorized-db)
  (if (> (count vectorized-db) 0)
    (recur waterfall-db vectorized-db) waterfall-db)))

The program compiles, but seems to be looping infinitely. I did a test making the loop quit after one iteration, and it returned an empty map (it should have had one item in it). Clearly I'm improperly associating items with the map, and it makes me think I must be disassociating items with the vector improperly as well. I can't see where I'm going wrong though—have I scoped my locals improperly?

2
clojure
Please, can you put an example of the intended use? - Juan Manuel

2 Answers

7
votes

In clojure, data structures don't change in-place: new data-structure values are calculated from old ones.

  • In your code, you do (pop vectorized-db) with the intention of popping the last element out. Clojure's pop doesn't work like this, but computes a new vector with the same elements as vectorized-db but the last one.
  • The same applies to (assoc waterfal-db key val).

What you have to do is pass the new values in the recur call. The code (which I haven't tested because I don't have an example of what it is supposed to do) transforms to:

(defn map-builder
  "Transforms the vector of waterfalls into a map of waterfalls."
  [input]
  (loop [waterfall-db {}
         vectorized-db input]
    (if (empty? vectorized-db)
      waterfall-db
      (let [key-str (last vectorized-db)
            key (->> key-str
                     (re-seq #"[0-9]+")
                     keyword)
            val (subs key-str (+ 2 (.indexOf key-str ":")))]
        (recur (assoc waterfall-db key val) (pop vectorized-db))))))
6
votes

The answer posted by Juan Manuel is correct, but you might consider building your map via into which is more straightforward:

(defn- make-kv [s]
    (let [key (->> s (re-seq #"[0-9]+") first keyword)
          val (-> s (.split ":") second string/trim)]
    [key val]))

(defn map-builder [v] 
    (into {} (map make-kv v)))