Deatomizing a map

Question

I have atoms in maps, which may or may not be a good idea, but the point is I needed to deref the atoms so I could json-str the maps, and json-str can't handle atoms, so I wrote this:

(defn deatomize- [m]
 (cond
   (instance? clojure.lang.Atom m) #(deatomize- @m)
   (map? m) (zipmap (keys m) (map #(trampoline deatomize- %) (vals m)))
   :else m
 )
)

(defn deatomize [m] (trampoline deatomize- m))

which seems to work, but a) is it good, b) is there a better way ?

Answer 1

I think your code will work fine.

Some general feedback:

• atoms in maps are a bit of an anti-pattern - usually you try to put maps in atoms. The idea is to keep your data structures immutable as much as possible, and have your references hold entire data structures at a fairly granular level. I'd strongly suggest rethinking this design - it will probably hurt you in the long run (for example, most Clojure library functions will assume immutable maps)
• you are not deatomising the keys. maybe you never use atoms in keys in which case this is fine, but thought it was worth pointing out.
• normally in Clojure closing parentheses don't get their own line and go at the end of the preceding line. this might feel odd at first, but it is good Lisp style that you should probably adopt.
• performance of this function might not be that great because it is rebuilding the entire map structure piece by piece, even if nothing changes. reduce can be used to improve this, by only altering the map when you need to.
• I think it is better to test for clojure.lang.IDeref rather than clojure.lang.Atom. By doing this, your code will also handle other reference types if needed.
• trampoline is going to add overhead and is only needed in the (presumably rare?) case where you have really deep nesting in your data structure that is likely to cause stack overflows. If you need both safe stack handling and good performance then you might consider a recursive implementation and falling back to a trampoline version in the case of a StackOverflowException.
• if you find an atom, the function is actually tail-recursive so you can use recur. this will reduce the need to use trampoline!

Here's an alternative to consider:

(defn deatomize [m] 
  (cond
    (instance? clojure.lang.IDeref m) 
      (recur @m)
    (map? m) 
      (reduce 
        (fn [current-map [k v]] 
          (let [nv (deatomise v)] 
            (if (= v nv) current-map (assoc current-map k nv)))) 
        m 
        m)
    :else m))