CODEWITHSUNDEEP
clojure
Rashi Bhalla
Rashi Bhalla

Reputation: 25

loop/recur is producing infinite abrupt results while the answer is correct for 1st value of collection

I have a list of values and I am performing some calculation on the first value of list and storing the result in a data structure.

I am new to clojure and I think problem lies in running the loops

(defn cal-funct [mean-value ,st-dev ,values]

 (def pi 3.14159)
(def e 2.71828)

(loop [f [] 
      values values
     ]
(if (not-empty values)
(do
(println str "first " (first values))
(let [y (* (/ 1 (* st-dev (Math/sqrt (* 2 pi)))) (Math/pow e (* -1 (/ (Math/pow (- (first values) mean-value) 2) (* 2 (Math/pow st-dev 2))))))
]
(println str "y is" y)
(recur (rest values) (conj f y))
)
)

f
)
)
 )

y should be calculated for all values of #values while its running infinite..

Upvotes: 0

Views: 73

Answers (2)

Svante
Svante

Reputation: 51561

There are several problems with your code. The first is that it is not formatted at all. Clojure has a fairly universally accepted style guide with only few variations, e. g. documented here. Not formatting code leads to incredibly hard to see, but in essence very simple, bugs.

Here is your code, formatted:

(defn cal-funct [mean-value ,st-dev ,values]
  (def pi 3.14159)
  (def e 2.71828)
  (loop [f [] 
         values values]
    (if (not-empty values)
      (do
        (println str "first " (first values))
        (let [y (* (/ 1
                      (* st-dev
                         (Math/sqrt (* 2 pi))))
                   (Math/pow e
                             (* -1
                                (/ (Math/pow (- (first values) mean-value)
                                             2)
                                   (* 2 (Math/pow st-dev 2))))))]
          (println str "y is" y)
          (recur (rest values)
                 (conj f y))))
      f)))

In no particular order:

  • Don't use def in a function body, use let instead. In this case, it seems that you want pi and e to be global constants, so define them outside.
  • Comma , is whitespace. It serves no purpose in a parameter list like that.
  • str is a function. I do not think that you want to print it.
  • After formatting, you can see that the order of recur arguments is not the same as in the loop bindings.
  • The function name should reflect what is calculated.

Some more improvements:

  • (/ 1 whatever) can be expressed as (/ whatever).
  • (* -1 whatever) can be expressed as (- whatever).
  • Maybe extract the calculation of y.
  • This kind of loop-recur is better expressed as a mapv.

Upvotes: 0

Sean Corfield
Sean Corfield

Reputation: 6666

You have your arguments to recur the wrong way round. You're binding (rest values) to f and (conj f y) to values in the recur call, so values will never be empty.

Upvotes: 3

Related Questions