In Clojure, How do I update a nested map correctly?

Question

I've just started learning Clojure, after many years of Java (and PHP/JavaScript) experience. What a challenge :-)

How do I update a map of values idiomatically? When I use the map function on a map it doesn't return a map, it returns a sequence.

I'm working on a small app where I have a list of tasks. What I'd like to do is alter some of the values in some of the individual tasks, then update the list of original tasks. Here are the tasks I'm testing with:

(defrecord Task [key name duration])

(def tasks
  (atom
    {
     "t1" (->Task "t1" "Task 1" 10)
     "t2" (->Task "t2" "Task 2" 20)
     "t3" (->Task "t3" "Task 3" 30)
     }
    ))

I've put the tasks in a hashmap, using a string key so it has fast, direct access to any task in the map. Each task holds the key as well, so I know what it's key is when I'm passing individual tasks to other functions.

To update the durations I'm using map and update-in to iterate over and selectively update the duration of each task, and returning the modified tasks.

Here's the function:

(defn update-task-durations
  "Update the duration of each task and return the updated tasks"
  [tasks]
  ; 1) Why do I have to convert the result of the map function,
  ;    from a sequence then back to a map?
  (into {}
    (map
      (fn [task]
        (println task) ; debug
        (update-in
          task
          ; 2) Why do I have to use vector index '1' here
          ;    to get the value of the map entry?
          [1 :duration]
          (fn [duration]
            (if (< duration 20)
              (+ duration 1)
              (+ duration 2)
              )
            )
          )
        ) tasks))
  )

I print the before/after values with this:

(println "ORIGINAL tasks:")
(println @tasks)

(swap! tasks update-task-durations)

(println "\nUPDATED tasks:")
(println @tasks)

1) The main problem I'm having is that the map function returns a sequence, and not a map, so I'm having to convert the sequence back to a map again using into {} which seems to me to be unnecessary and inefficient.

Is there a better way to do this? Should I be using a function other than map?

Could I arrange my data structures better, while still being efficient for direct access to individual tasks?

Is it ok to convert a (potentially very large) sequence to a map using into {} ?

2) Also, inside my function parameter, that I pass to the map function, each task is given to me, by map, as a vector of the form [key value] when I would expect a map entry, so to get the value from the map entry I have to pass the following keys to my update-in [1 :duration] This seems a bit ugly, is there a better/clearer way to access the map entry rather than using index 1 of the vector?

Answer 1

A popular way to solve this mapping-over-maps problem is with zipmap:

(defn map-vals
  "Returns the map with f applied to each item."
  [f m]
  (zipmap (keys m)
          (map f (vals m))))

(defn update-task-durations
  [tasks]
  (let [update-duration (fn [duration]
                          (if (< duration 20)
                            (+ 1 duration)
                            (+ 2 duration)))]
    (->> tasks
         (map-vals #(update % :duration update-duration)))))

(swap! tasks update-task-durations)

For Clojure < 1.7, use (update-in % [:duration] ... instead.

Alternatively, you could also use destructuring to simplify your current solution without defining a utility function:

(->> tasks
     (map (fn [[k task]]
            [k (update task :duration update-duration)]))
     (into {})

Why?

map only deals with sequences. If you're into type signatures, this means that map always has the same type (map :: (a -> b) -> [a] -> [b]), but it also means that all you'll get out of map is a seq-of-something.

map calls seq on its collection parameter before doing anything, and seq-ing a map gives you a sequence of key-val pairs.

Don't worry too much about efficiency here. into is fast and this is pretty idiomatic.

Answer 2

Just get more alternatives: Instead of a map you can use a for

(into {}
   (for [[key value] your-map]
         [key (do-stuff value)]))

A faster way is reduce-kv

(reduce-kv 
   (fn [new-map key value] 
         (assoc new-map key (do-stuff value))) 
   {}
   your-map))

Of course you can also use a simple reduce

(reduce (fn [m key]
          (update m key do-stuff))
   your-map
   (keys your-map))