Dear Meikel and Ken,
Thank you very much for your corrections and suggestions! I admit it
took a while for me to understand the more sophisticated parts of your
code, but on the way I learned quite a lot about the correct usage of
conj, assoc, update-in, and fnil!
Best regards,
Stefan
On Jan 17, 3:21 pm, Ken Wesson <kwess...@gmail.com> wrote:
> On Mon, Jan 17, 2011 at 1:47 AM, Stefan Rohlfing
>
>
>
>
>
>
>
>
>
> <stefan.rohlf...@gmail.com> wrote:
> > Hi all,
>
> > I am trying to implement the function 'group-by' from Clojure.Core
> > using my current knowledge of Clojure but cannot get pass a
> > java.lang.Exception.
>
> > This is the code so far:
>
> > (defn key? [key coll]
> > (some #{key} (keys coll)))
>
> > (defn my-group-by [f coll]
> > (let [test (fn [m x]
> > (let [res (f x)]
> > (if (key? res m)
> > (conj (m res) x)
> > (assoc m res (vec x)))))]
> > (reduce test {} coll)))
>
> First of all, your key? is only ever (supposed to be) called on maps,
> so you can dispense with it:
>
> (defn my-group-by [f coll]
> (let [test (fn [m x]
> (let [res (f x)]
> (if (contains? res m)
> (conj (m res) x)
> (assoc m res (vec x)))))]
> (reduce test {} coll)))
>
> Second, your already-in-the-map case returns a vector. It doesn't
> modify the vector in place; it makes a new one with one more item and
> returns that. Not the map it should return.
>
> The next iteration of your reduce, the vector gets passed to key? with
> some value. Your key? function calls keys on it, which results in an
> exception. I get:
>
> user=> (keys ['x 'y 'z])
> java.lang.ClassCastException: clojure.lang.Symbol cannot be cast to
> java.util.Map$Entry
>
> You got:
>
> > ;; java.lang.Exception: Unable to convert: class java.lang.Integer to
> > Object[] (repl-1:2)
>
> I'm not sure why the difference -- different Clojure versions? -- but
> no matter, the fix is the same:
>
> (defn my-group-by [f coll]
> (let [test (fn [m x]
> (let [res (f x)]
> (if (contains? res m)
> (assoc m res (conj (m res) x))
> (assoc m res (vec x)))))]
> (reduce test {} coll)))
>
> This actually returns an updated map and not just an updated vector.
> But it can be made nicer in a couple of ways. One uses the optional
> not-found option to get to dispense with the contains?:
>
> (defn my-group-by [f coll]
> (let [test (fn [m x]
> (let [res (f x)]
> (assoc m res (conj (m res []) x))))]
> (reduce test {} coll)))
>
> And the other uses update-in. Update-in means the key only needs to be
> specified once, which lets you dispense with the inner let:
>
> (defn my-group-by [f coll]
> (let [test (fn [m x]
> (update-in m [(f x)] #(if % (conj % x) [x])))]
> (reduce test {} coll)))
>
> This can be formatted a bit more nicely:
>
> (defn my-group-by [f coll]
> (reduce
> (fn [m x]
> (update-in m [(f x)]
> #(if % (conj % x) [x])))
> {} coll))
>
> This gets rid of the outer let as well.
>
> The latter two versions can't distinguish the map having a key with an
> associated value of nil and the map lacking that key; however, the map
> never associates a key with anything other than a vector, so this ends
> up not mattering.
>
> For reference (and I only peeked at this *after* writing all of the
> above), the implementation of group-by in clojure.core is:
>
> (defn group-by
> "Returns a map of the elements of coll keyed by the result of
> f on each element. The value at each key will be a vector of the
> corresponding elements, in the order they appeared in coll."
> {:added "1.2"
> :static true}
> [f coll]
> (persistent!
> (reduce
> (fn [ret x]
> (let [k (f x)]
> (assoc! ret k (conj (get ret k []) x))))
> (transient {}) coll)))
>
> This is basically the same as the version I got adding the optional
> not-found argument to get instead of using update-in, except that it
> uses transients for the performance benefits (and has docstrings and
> metadata) and gets rid of the outer let in the same manner as the
> final update-in using version I wrote.
--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clojure@googlegroups.com
Note that posts from new members are moderated - please be patient with your
first post.
To unsubscribe from this group, send email to
clojure+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
