patternMinor
Number factorization in clojure
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
suffice. So it would overflow the stack for a number having too many
factors. However, since it uses
enough to do this.
factors.
You can do it with a single loop if you exhaust each prime factor as you go:
Now let's look at
use
fail to be prime.
testing more clearly (though it will run slower). Because these
are lazy, we don't do any redundant testing.
The result is
Edited to correct an erroneous comment.
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
recurwould
suffice. So it would overflow the stack for a number having too many
factors. However, since it uses
long arithmetic, no number is bigenough 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 thatfail to be prime.
- We can use
take-while,map, andnot-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.