Reducing "for" comprehension duplication

Question

In my answer to Clojure For Comprehension example, there is some duplication that I would like to remove:

(def all-letters (map char (range 65 90)))
(defn kw [& args] (keyword (apply str args)))
(concat
  (for [l all-letters] (kw l))
  (for [l all-letters l2 all-letters] (kw l l2))
  (for [l all-letters l2 all-letters l3 all-letters] (kw l l2 l3)))

I would like to remove the "for" duplication. I have written the following macro:

(defmacro combine [times]
 (let [symbols (repeatedly times gensym)
       for-params (vec (interleave symbols (repeat 'all-letters)))]
    `(for ~for-params (kw ~@symbols))))

Which works with:

 (concat (combine 1) (combine 2) (combine 3))

But if I try:

 (for [i (range 1 4)] (combine i))

I get:

CompilerException java.lang.ClassCastException: clojure.lang.Symbol cannot be cast to java.lang.Number, compiling:(NO_SOURCE_PATH:177)

What is going on? Is there a better way of removing the duplication?

mikera · Accepted Answer · 2012-07-17T23:24:43.483

1

Your problem is that combine is a macro that gets expanded at compile time. When you try to expand on a symbol i it fails, because it is designed to take a number (times). i is just a symbol at compile time, it only evaluates to numeric values at runtime.

I'd suggest rewriting combine to be a function rather than a macro: you don't really need macros here and functions are frequently more convenient (as in this case!).

Here's a recursive combine that probably does roughly what you want:

(defn combine
    ([times] (combine times nil))
    ([times letters]
      (if (<= times 0)
        (keyword (apply str letters))
        (flatten (for [l all-letters] (combine (dec times) (cons l letters)))))))

edited Jul 17 '12 at 23:24

answered Jul 17 '12 at 23:13

mikera

105,238
25
256
415

Is it because "for" is also a macro? – DanLebrero Jul 17 '12 at 23:17
the `for` isn't what is causing the problem, it is a macro but it has the effect you would expect at runtime - the real problem is the `i` which is a symbol at compile time when used in the `combine` macro. Added a few more explanatory notes. – mikera Jul 17 '12 at 23:25

score 1 · Answer 2 · answered Jul 18 '12 at 04:28

You can modify your macro such that the concat becomes part of the macro, such shown below. But I agree with Mikera that it is better to have a function.

(defmacro combine [start end]
 `(concat
      ~@(for [i (range start end)]
          (let [symbols (repeatedly i gensym)
             for-params (vec (interleave symbols (repeat 'all-letters)))]
          `(for ~for-params (kw ~@symbols))))))

user=> (combine 1 2)
(:A :B :C :D :E :F :G :H :I :J :K :L :M :N :O :P :Q :R :S :T :U :V :W :X :Y)

Reducing "for" comprehension duplication

2 Answers2