Thanks for that advice Alan. I've already run into a problem due to
"merge-with vector". I get your point about flatten, and I will
certainly look closely at your suggestions regarding partial.

Thanks again for taking the time.

Cheers!

On 4 April 2012 21:11, Alan Malloy <a...@malloys.org> wrote:
> On Apr 4, 6:50 am, David Jagoe <davidja...@gmail.com> wrote:
>> Particularly I very often find myself doing
>>
>> (apply hash-map (flatten (for [[k v] some-map] ...)))
>
> :( :( :( flatten is vile, never use it[1]. What if the value in the
> map is a list, or the key is a vector? Now you've broken it. Instead
> use into:
>
> (into {}
>      (for [[k v] some-map]
>        [k (f v)])) ;; or whatever k/v pair you want
>
> I'm also very surprised to see (apply merge-with vector ...), because
> this seems like it is rarely right. If you have three maps, {:a 1},
> {:a 2}, and {:a 3}, do you really want the result to be {:a [[1 2]
> 3]}? You're probably better off converting to {:a [1]} {:a [2]} {:a
> [3]} beforehand and using (apply merge-with into ...).
>
> Your implementation of selected? is also strange - why are you
> iterating through the keys, instead of using their fast lookup? I'd
> write (selected? [coll] (contains? select coll)). But that brings up
> another point: selected? and agg are just partial applications of
> contains and get:
>
> (let [selected? (partial contains? select)
>      agg (partial get select)]
>  ...)
>
> Or, if you prefer never to repeat anything:
>
> (let [[selected? agg] (for [f [contains? get]]
>                        (partial f select))]
>  ...)
>
> [1] technically you want it sometimes, but I strongly suggest staying
> away.
>
>>
>> Any pointers or advice on improving my style will be much appreciated!
>>
>> (defn aggregate
>>   "Aggregate rows using the functions provided in select and after
>>    filtering the rows by where.
>>
>>     E.g.
>>
>>       (aggregate [{:a 1 :b 2} {:a 3 :b 4} {:a 5 :b 6}] :select {:a +}
>> :where (fn [r] (< (:b r) 6))) --> {:a 4}"
>>
>>     [rows & {select :select where :where}]
>>     (letfn [(selected? [col] (some #{col} (keys select)))
>>              (agg [col] (get select col))]
>>       (apply hash-map
>>              (flatten
>>               (for [[k vs] (apply merge-with vector (filter where
>> rows)) :when (selected? k)]
>>                 [k (apply (agg k) vs)])))))
>>
>> Thanks,
>>
>> --
>> David Jagoe
>>
>> davidja...@gmail.com
>> +447535268218
>
> --
> 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



-- 
David Jagoe

davidja...@gmail.com
+447535268218

-- 
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

Reply via email to