5

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?

Steve Moseley
  • 5,797
  • 2
  • 19
  • 22

2 Answers2

3

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.

BenC
  • 451
  • 3
  • 7
  • Thanks for your answer. I like the idea of a utility function, taking an update function as a parameter, making the map/update more general purpose. zipmap looks very powerful, but I'm a bit wary about how it extracts all the key, all the values, then builds a new map - is this an efficient way to implement this? (all my experience with Java is telling me to avoid this extracting and rebuilding !) – Steve Moseley Dec 20 '15 at 23:11
  • @SteveMoseley The most performant way is likely combining `reduce-kv` with [transients](http://clojure.org/transients) -- you can do basic timings in the REPL with `(time ...)` to experiment, and use `(source ...)` to view the source. But `keys` and `vals` both just return iterators (`seq`s) backed by the original map, and `zipmap` is essentially `(assoc map k v) while hasNext(key and val)`, to totally abuse Java-ish syntax. It ends up fairly efficient anyway because Clojure is optimized for this -- there's a lot of structural sharing in the background between all these immutable objects. – BenC Dec 20 '15 at 23:46
  • Transients sound like an excellent facility to have available, thanks for the link. As I'm still very new to Clojure, perhaps I should try to focus on understanding the functional programming approach a bit more, and leave issues of performance until it's really needed :-) – Steve Moseley Dec 21 '15 at 00:16
  • @SteveMoseley Adapting your Java experience is a learning process for sure! Note that transients are still immutable, they just have faster versions of `assoc` and friends. I definitely agree that it's best to just get more comfortable with the Clojure idioms and approach first, and keep reminding yourself that a lot of these operations that feel pretty heavy aren't actually doing very much internally. Have fun! :) – BenC Dec 21 '15 at 00:30
3

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))  
user5187212
  • 426
  • 2
  • 7
  • Wow - three alternative ways to do it, nice! I hadn't realised that 'reduce' could be used this way, or even 'for', it makes me realise I've got a long way to go before I can truly be confident that I know what I'm doing with Clojure – Steve Moseley Dec 20 '15 at 23:12