Did some code review yesterday on some recent Clojure code and wanted to list one simple example of some ugly stuff we cleaned up a bit.
I had some code like this:
(defn check-prop-bridges [model] (let [class-maps (class-map-names model) prop-bridges (prop-bridge-mapping model)] (map #(bad-class-map-err (first %) (second %)) (filter #(not (is-class-map? (second %) class-maps)) prop-bridges))))
I’m skipping lots of supporting code here but in essence the
class-map-names function is returning a list of all known names and
prop-bridge-mapping is returning a list of vectors in the form
[prop-bridge-name class-map-name]. And the point of this code is to look through all of the mappings, find mappings where the class-map-name is unknown, and generate an error for each of those.
I was accomplishing that by first doing a
filter to remove good mappings, then using
map to generate an error for each remaining bad mapping.
The main thing that was smelly here was all the
second business. 1) it’s kind of ugly and verbose, 2) there are two functions relying on the positional nature of the data returned, 3) this code is implicitly coupled to the
prop-bridge-mapping function creating that intermediate data structure.
The key insight in cleaning this up a bit was to use destructuring in a list comprehension (
for) to make the ordering of the passed data more explicit AND perform the filter and map in a single conceptual step:
(for [[p c] prop-bridges :when (not (is-class-map? c class-maps))] (bad-class-map-err p c))))
The list comprehension has the documentation:
user=> (doc for) ------------------------- clojure.core/for ([seq-exprs body-expr]) Macro List comprehension. Takes a vector of one or more binding-form/collection-expr pairs, each followed by zero or more modifiers, and yields a lazy sequence of evaluations of expr. Collections are iterated in a nested fashion, rightmost fastest, and nested coll-exprs can refer to bindings created in prior binding-forms. Supported modifiers are: :let [binding-form expr ...], :while test, :when test.
That is, something like:
(for [stooge ["Larry" "Curly" "Moe"]] (hit-with-pipe stooge))
can be read “for each
stooge in this sequence of stooges, perform the
hit-with-pipe action.” In the [ ] you see the variable and the sequence from which it is taken (and there can be multiple variables as well).
Going back to our refactored code above, we see that the variable is not just a mapping but actually
[p c]. This is destructuring and actually defines the structure of the expected mapping AND binds the first and second elements to
Additionally, the filtering operation is occurring during the list comprehension using a
:when modifier. The
body-expr will only be evaluated when the :when modifier returns true.