1
\$\begingroup\$

Am I doing it right? Is there some function in the standard library I should be using? This is my first real(ish) world Clojure program…

Input

(def data
  [[{:id 5 :name "a"} {:id 1 :name "b"}], [{:id 4 :name "c"} {:id 1 :name "d"}]])

Code

The elements are maps and they are compared based on their :id.

(def precomputed
  (reductions
   (fn [global, v]
     (let [seen? (:seen? global)
           new   (remove #(-> % :id seen?) v)
           old   (filter #(-> % :id seen?) v)]
       {:seen? (clojure.set/union seen? (->> new (map :id) (set)))
        :new   new
        :old   old}))
   {:seen? #{} :new '() :old '()}
   data))

At this moment precomputed looks like

({:seen? #{}, :new (), :old ()}
{:seen? #{1 5}, :new ({:id 5, :name "a"} {:id 1, :name "b"}), :old ()}
{:seen? #{1 4 5}, :new ({:id 4, :name "c"}), :old ({:id 1, :name "d"})})

Print some sort of output

(doseq [i (rest precomputed)]
  (println "new: " (->> (i :new) (map :name) (interpose ", ") (reduce str))
           "old: " (->> (i :old) (map :name) (interpose ", ") (reduce str))))

Output

new:  a, b old:     ; first vector, every element is new
new:  c old:  d     ; we have seen element with :id 1 before
\$\endgroup\$
  • \$\begingroup\$ Does this all work as intended? If so, what would you like reviewed? \$\endgroup\$ – Jamal Oct 26 '14 at 23:34
  • \$\begingroup\$ @Jamal Uh. Am I doing it right? Is there some function in the standard library I should be using? This is my first real(ish) world Clojure program… \$\endgroup\$ – user7610 Oct 26 '14 at 23:37
  • 1
    \$\begingroup\$ You can put that into the question, then. \$\endgroup\$ – Jamal Oct 26 '14 at 23:38
  • \$\begingroup\$ You could probably use partition-all to partition the input into the two outputs. \$\endgroup\$ – ferada Oct 27 '14 at 11:03
  • 1
    \$\begingroup\$ split-with wouldn't work here, as it is equivalent to take-while/drop-while and not filter/remove. What you really looking for is probably group-by, such that (let [m (group-by #(-> % :id seen?) v), old (m true), new (m false)] ...) \$\endgroup\$ – abuzittin gillifirca Oct 30 '14 at 11:19
1
\$\begingroup\$

Concatenating a sequence of strings

You should use (apply str) instead of (reduce str) As str can take multiple arguments, and apply can take a sequence as a second argument. Intermediate results generated by reduce would cause time and space waste.

Since you are using a separator string, use clojure.string/join instead:

(->> (i :new) (map :name) (clojure.string/join ", "))

Naming loop vars

In the above code i is not a good name, I expect i to be some kind of index. If using meaningful names like user, order is not available; you can user curr for current m for map as temporary variables. Important point is principle of least surprise.

Map destructuring

You can also use map destructuring. It is easiest to show it by example:

> (doseq [{:keys [new old]} [{:new 1 :old 2} {:new 3 :old 4}]] (prn old new old))
2 1 2
4 3 4

So similarly here:

(doseq [{:keys [new old]} (rest precomputed)]
  (println "new: " (->> new (map :name) (clojure.string/join ", "))
           "old: " (->> old (map :name) (clojure.string/join ", "))))

Prefer functions over constants

Prefer functions over data. Functions are the basic unit of re-use in clojure. It will be obvious if you try to test precomputed you provided, with multiple example data (in REPL by hand or with run-tests). Other signs precomputed should be a function is:

  • it's name: precomputed implies a function by name precompute should exist (also/instead).

  • data is only referred to once, right at the end; and the rest of the form doesn't depend on it.

Decompose precomputed

See values in precomputed does not depend on the previous values of :new and :old but only those of :seen?.

It can be rewritten thus:

(defn seen-ids [data]
  (reductions
      #(clojure.set/union %1 (->> %2 (map :id) set))
    #{}
    data))

You don't need to put parens around set.

You can then get new and old elements separately:

(defn new-elts [seen-ids data]
  (map (fn [seen? v] (remove #(-> % :id seen?) v)) seen-ids data))

Terse function definition

Sometimes you may want to define a function as combination of other functions instead of using fn. Combinators clojure has for this purpose are comp, partial, and juxt.

For example #(-> % :id seen?) is equivalent to (comp %1 :id), which also allows the fn to be replaced by #. You can sometimes use partial to define functions w/o using parameter names, but I do not recommend it to use in top level functions, as parameter names are important developer documentation; but this style may be useful for shortening inline definitions (in let, for etc).

old-elts can be defined using this style, like this (compare with the definition of new-elts above):

(def old-elts (partial map #(filter (comp %1 :id) %2)))
\$\endgroup\$
1
\$\begingroup\$

Tackling the problem afresh, you can accomplish the heart of the computation as follows:

(let [ids (map (partial map :id) data)
      seens (reductions into #{} ids)
      classes (map (fn [s ms] (group-by (comp nil? s :id) ms)) seens data)]
  (map
   (partial map-kv (fn [k v] [({true :new, false :old} k) (map :name v)]))
   classes))

... which produces

({:new ("a" "b")} {:new ("c"), :old ("d")})

To see how it works, look at ids, seens, and classes for a number of values of data.

All the functions employed are standard, except for

(defn map-kv
  "Returns the map resulting from applying the function
  f to each of the entries of map m. f must return a 
  key-value pair (a two-element sequence)."
  [f m]
  (into {} (map (fn [[k v]] (f k v)) m)))

... which is lifted from the Clojure Cookbook, section 2.21. I've swapped the arguments around to comply with the convention that collection arguments figure last in map and its variants.

This lazy version is likely to be quite a bit slower than one using loop/recur.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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