Why do I get an error from this code?

Question

I´m new to clojure and am trying to break through some of the walls I keep running into. The code in question is the function v3 which should accept 4 arguments:

• a min and a max integer, mi and ma, to use with the random-numbers function to find numbers within a certain range,
• another integer,cnt, to signify how many numbers I want in my final list, and
• tones, which is a list of integers that the randomized numbers have to match once I've calculated modulo 12 of said numbers.

The function should run until o is a list of length cnt containing random numbers that are also in the tones list.

My document compiles just fine but when I want to run the function itself in a repl, for example using something like (v3 58 52 15 '(0 2 4 5 7 9)) I get the following error:

ClassCastException clojure.langLazySeq cannot be cast to java.lang.Number clojure.langNumbers.reminder (Numbers.java:173)

Here's my code

(defn random-numbers [start end n]
    (repeatedly n #(+ (rand-int (- end start)) start)))

(defn m12 [input]
    (mod input 12))

(defn in? [coll elm]  
    (some #(= elm %) coll))

(defn v3 [ma mi cnt tones]
    (let [o '()]
        (loop []
            (when(< (count o) cnt)
                (let [a (m12 (random-numbers mi ma 1))]
                    (if (in? tones a)
                        (conj o a)))))
        (println o)))

Answer 1

First of all, it is more idiomatic Clojure to type the parentheses on the same line, and not in the "Java"-way.

When I debug your code I see it fails at the call to m12: random-numbers returns a sequence and the call to mod in m12 expects a number.

You can fix this issue by for example taking the first element from the sequence returned by random-numbers:

(defn v3
  [ma mi cnt tones]
  (let [o '()]
    (loop []
      (when (< (count o) cnt)
        (let [a (m12 (first (random-numbers mi ma 1)))]
          (if (in? tones a)
            (conj o a)))))
    (println o)))

/edit

I am not sure what your code is supposed to be doing, but this did not stop me to make some more changes. If you use a loop, you usually also see a recur to "recur" back to the loop target. Otherwise it does not do much. I added the following things:

1. a recur to the loop.
2. The let statement added to the loop vector (starting value).
3. println statements in the false clause of the if-statement.
4. Removed the first if-statement that checked the count
5. Changed list to vector. You would use a list over a vector when you create code structures structure (for example while writing macros).

See:

(defn v3
  [ma mi cnt tones]
  (loop [o []]
    (if (< (count o) cnt)
      (let [a (m12 (first (random-numbers mi ma 1)))]
        (if (in? tones a)
          (recur (conj o a))
          (println "a not in tones, o:" o)))
      (println "already " cnt "tones generated"))))

If you run (v3 58 52 4 [0 2 4 5 7 9]) (note I changed your 15 for cnt to 4 and changed the list to a vector) a few times you get for example the following output:

a not in tones, o: [4 4]
a not in tones, o: [9 5 5]
a not in tones, o: []
already 4 tones generated
a not in tones, o: [7]

Hope this helps.

Answer 2

I think I see what you are trying to do.

This is an exercise in automatic composition. Your v3 function is intended to generate a sequence of tones

• in a range given by min and max.
• with tone class drawn from a given set of tone classes (tones)

The m12 function returns the tone class of a tone, so let's call it that:

(defn tone-class [tone]
  (mod tone 12))

While we're about it, I think your random-number function is easier to read if we add the numbers the other way round:

(defn random-number [start end]
  (+ start (rand-int (- end start))))

Notice that the possible values include start but not end, just as the standard range does.

Apart from your various offences against clojure semantics, as described by @Erwin, there is a problem with the algorithm underlying v3. Were we to repair it (we will), it would generate a sequence of tone classes, not tones. Interpreted as tones, these do not move beyond the base octave, however wide the specified tone range.

A repaired v3

(defn v3 [mi ma cnt tones]
  (let [tone-set (set tones)]
    (loop [o '()]
      (if (< (count o) cnt)
        (let [a (tone-class (random-number mi ma))]
          (recur (if (tone-set a) (conj o a) o)))
        o))))

• For a start, I've switched the order of mi and ma to conform with range and the like.
• We turn tones into a set, which therefore works as a membership function.
• Then we loop until the resulting sequence, o, is big enough.
• We return the result rather than print it.

Within the loop, we recur on the same o if the candidate a doesn't fit, but on (conj o a) if it does. Let's try it!

(v3 52 58 15 '(0 2 4 5 7 9))
;(4 5 9 7 7 5 7 7 9 7 5 7 4 9 7)

Notice that neither 0 nor 2 appears, though they are in tones. That's because the tone range 52 to 58 maps into tone class range 4 to 10.

Now let's accumulate tones instead of tone classes. We need to move conversion inside the test, replacing ...

        (let [a (tone-class (random-number mi ma))]
          (recur (if (tone-set a) (conj o a) o)))

... with ...

        (let [a (random-number mi ma)]
          (recur (if (tone-set (tone-class a)) (conj o a) o)))

This gives us, for example,

(v3 52 58 15 '(0 2 4 5 7 9))
;(53 52 52 52 55 55 55 53 52 55 53 57 52 53 57)

An idiomatic v3

An idiomatic version would use the sequence library:

(defn v3 [mi ma cnt tones]
  (let [tone-set (set tones)
        numbers (repeatedly #(random-number mi ma))
        in-tones (filter (comp tone-set tone-class) numbers)]
    (take cnt in-tones)))

This generates the sequence front first. Though you can't tell by looking at the outcome, the repaired version above generates it back to front.

An alternative idiomatic v3

Using the ->> threading macro to capture the cascade of function calls:

(defn v3 [mi ma cnt tones]
  (->> (repeatedly #(random-number mi ma))
       (filter (comp (set tones) tone-class))
       (take cnt)))