4
\$\begingroup\$

This is my implementation of Clojure's assoc-in function. I am looking for tips on making it more idiomatic and in general, better.

(defn my-assoc-in
  [m [& ks] v]
  (let [sorted-keys (reverse ks)]
    (loop [curr-key (first sorted-keys)
           rem-keys (rest sorted-keys)
           curr-map (get-in m (reverse rem-keys))
           acc-val  (assoc curr-map curr-key v)]
      (if (empty? rem-keys)
        acc-val
        (let [next-key (first rem-keys)
              next-rem (rest rem-keys)
              next-map (get-in m next-rem)
              next-val (assoc next-map next-key acc-val)]
          (recur next-key next-rem next-map next-val))))))
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$
           curr-map (get-in m (reverse rem-keys))

That seems expensive (linear in number of keys).

At this point, isn't (reverse rem-keys) simply (rest ks) ? That's available in O(1) rather than O(N) time.


           acc-val  (assoc curr-map curr-key v)]
      (if (empty? rem-keys)
        acc-val
        (let ...

We unconditionally create an assoc result. But we only use it in the base case where there's no more keys.

Prefer to use that expression directly, eliding the acc-val symbol:

      (if (empty? rem-keys)
        (assoc curr-map curr-key v)
        (let ...

I get that you were shooting for parallel structure with the four let variables. The next-val down there is very nice and should be retained since it reads smoothly.

BTW, even if you keep all eight identifiers, there's an opportunity to rename to curr-rem and curr-val if you want to call out the parallel structure.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.