Prime factors kata solution in Clojure

Question

I'm new to Clojure but I already know about functions and lists, so I decided to implement Prime Factors Kata solution. For any natural number the function should return a list of all its prime factors, e.g. for 15 it should return (3 5).

It turned out that to implement a stack safe function I needed to use loop and recur, which I was not familiar with.

So, does my solution look like idiomatic Clojure? What could be improved? Thanks!

(defn primeFactorsOf
 "Return all prime factors of a number"
 [n]
 (loop [number n candidate 2 factors ()]
   (if (= number 1)
     (reverse factors)
     (if (= (mod number candidate) 0)
       (recur (/ number candidate) candidate (conj factors candidate))
       (recur number (inc candidate) factors)))))

Answer 1

There's a few notable things here:

You're prepending to a list, then reversing the result at the end. It would be better to just start with a vector ([]). conjoining to a vector appends the element to the end instead, so you don't need to reverse it after.

If you really want a list here because it's checking for type, then either ignore this suggestion, or put this code into a helper and do something like:

(def primeFactorsOf [n]
  (list* (helper n)))

I personally think a vector makes more sense here, but it may prove to not be worth it in this case.

---

(= (mod number candidate) 0)

can just be

(zero? (mod number candidate))

---

I wouldn't have all your loop accumulators on the same line like you have. It's much harder to read, since you have to manually group them in two's in your head to make sense of them. I'd split them up:

(loop [number n
       candidate 2
       factors []]
  . . .)

If you really want to have them on one line though, add some commas in there. Commas have, as far as I know, no purpose other to allow you to group expressions to help readability. I'd add them in every two here:

(loop [number n, candidate 2, factors []]
  . . .)

---

Nested if expressions are usually cleaner using a cond. There are a multitude of ways to format a cond block, but when you have long lines like you have here, I like to format it almost like you would an if statement in a imperative language (below).

---

All together, I end up with:

(defn primeFactorsOf
 "Return all prime factors of a number"
 [n]
 (loop [number n
        candidate 2
        factors []]
   (cond
     (= number 1)
     factors

     (zero? (mod number candidate))
     (recur (/ number candidate) candidate (conj factors candidate))

     :else
     (recur number (inc candidate) factors))))