CODEWITHSUNDEEP
macrosclojurecode-duplicationfor-comprehension
DanLebrero
DanLebrero

Reputation: 8593

Reducing "for" comprehension duplication

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?

Upvotes: 2

Views: 118

Answers (2)

Ankur
Ankur

Reputation: 33657

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)

Upvotes: 1

mikera
mikera

Reputation: 106381

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

Upvotes: 1

Related Questions