Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Divisor function in clojure

I'm new to clojure and I want to create a function which returns a vector of all divisors of a certain Number.

For instance:
[1 2 3] for 6 as input

(defn div [x seq]
  (let [r  (range 1 (+ (/ x 2) 1)) ]   
    (dotimes [i (count (range 1 (+ (/ x 2) 1) ))]      
     (cond
      (= (mod x (nth r i )) 0)(print (conj  seq (nth r i ))) 
     ))))

This function returns the output in the following format:
[1][2][4][5][10][20][25][50] for 100 but I want to get the output in one vector. It seems that the seq variable is constantly overwritten with each loopstep. Could anyone explain this behaviour and provide me a workaround?

Thanks in advance and best regards

like image 606
Hellekin Avatar asked Nov 27 '13 16:11

Hellekin


3 Answers

You can avoid looping using a somewhat more idiomatic solution:

(defn divisors 
  [n]
  (filter (comp zero? (partial rem n)) (range 1 n)))
like image 116
guilespi Avatar answered Nov 01 '22 09:11

guilespi


I think your approach is not correct, you can take a look to:

  • the loop function to resolve this kind of problems (i mean the cases where you need iterate and yield value),
  • iterate function
  • recursive functions (a function that calls itself)

And this code (using 'for' function ) can easily resolve your specification

(let [n 100]
  (for [x (range 1 n)
       :when (zero? (rem n x))]
   x))
=>(1 2 4 5 10 20 25 50)
like image 4
tangrammer Avatar answered Nov 01 '22 10:11

tangrammer


Your fundamental problem is that you are attempting to take an imperative approach, but Clojure collections are immutable. Also, I think that dotimes will always return nil, and print returns nil after printing its parameter(s).

There is a better way, but let's first see how we can use atoms to obtain an imperative solution:

(defn div [x seq]
  (let [r  (range 1 (+ (/ x 2) 1)) ]   
    (dotimes [i (count (range 1 (+ (/ x 2) 1) ))]      
      (cond
       ;; "Append" the ith element of r to seq
       ;; (Technically, we are replacing the value stored in seq
       ;; with a new list -- the result of conj-ing (nth r i)
       ;; to the current value stored in seq)
       (= (mod x (nth r i )) 0) (swap! seq conj (nth r i )))))  ;; <= don't print
  seq) ;; <== seq is what we're interested in, so we return it here.
       ;;     Otherwise, we would return the result of dotimes,
       ;;     which is nil

Note that we have eliminated the print and expect seq to be an atom (which we update using swap!). We can now call div as follows:

user> (deref (div 6 (atom [])))
[1 2 3]

We could improve this by moving seq from the parameter list to a let inside the function, then dereferencing it when we return. But it would be better to avoid mutation in the first place. As tangrammer indicates in his answer, this is easily accomplished using for:

(defn div [x]
  (let [r (range 1 (+ (/ x 2) 1))]
    ;; Loop over r, taking only elements that are divisors of x
    (for [i r
          :when (= (mod x i) 0)]
      i))) ;; <= for accumulates the result of this expression (here simply i) into a sequence

user> (div 6)
(1 2 3)

In production code, you would probably inline r in this case, but I wanted to retain your original structure as much as possible. We could also use zero? in the :when clause. But all we're really doing in the for loop is a simple filter, so we could take Guillermo's approach and use filter instead.

like image 2
Nathan Davis Avatar answered Nov 01 '22 11:11

Nathan Davis