HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Number factorization in clojure

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
numberclojurefactorization

Problem

I'm just a Clojure noob (started learning yesterday). Here is my helloworld program. It factorizes number into primes. Do you have any comments? How could I make this code cleaner and shorter? Maybe I can deduplicate some things?

(defn lazy-primes
([] (cons 2 (lazy-seq (lazy-primes 3 [ 2 ]))))
([current calculated-primes]
(loop [ [first-prime & rest-primes] calculated-primes]
(if (> (* first-prime first-prime) current)
(cons current (lazy-seq (lazy-primes
(inc current)
(conj calculated-primes current))))
(if (= 0 (mod current first-prime))
(lazy-seq (lazy-primes (inc current) calculated-primes))
(recur rest-primes))))))

(defn factorize
([num] (factorize num '(1) (lazy-primes)))
([num acc primes]
(if (= num 1) acc
(loop [ [head & rest] primes ]
(if (= 0 (mod num head))
(factorize (/ num head) (cons head acc) primes)
(recur rest))))))

Solution

Wow! If this is you after one day, we hope for great things.

However, you can make your program simpler. Let's start with factorize. It has a couple of defects:

  • It uses proper recursion where tail recursion with recur would


suffice. So it would overflow the stack for a number having too many
factors. However, since it uses long arithmetic, no number is big
enough to do this.

  • Each recursive call retries all the primes that have failed to be


factors.

You can do it with a single loop if you exhaust each prime factor as you go:

(defn factorize [num]
(loop [num num, acc [1], primes (lazy-primes)]
(if (= num 1)
acc
(let [factor (first primes)]
(if (= 0 (mod num factor))
(recur (quot num factor) (conj acc factor) primes)
(recur num acc (rest primes)))))))


Now let's look at lazy-primes. While sticking to your algorithm ...

  • We can simplify the base case - the one with no arguments.



  • You produce a lazy sequence level for every number tested. We can


use recur to short-circuit tail recursion for all the numbers that
fail to be prime.

  • We can use take-while, map, and not-any? to express the prime


testing more clearly (though it will run slower). Because these
are lazy, we don't do any redundant testing.

The result is

(defn lazy-primes
([] (lazy-primes 2 []))
([current known-primes]
(let [factors (take-while #(

A couple of arbitrary changes:

  • I've used the abbreviated #(...) function syntax.



  • I renamed calculated-primes to known-primes`.



Edited to correct an erroneous comment.

Context

StackExchange Code Review Q#114619, answer score: 9

Revisions (0)

No revisions yet.