CODEWITHSUNDEEP
recursionclojurelisp
JKRT
JKRT

Reputation: 1207

Summing all the multiples of three recursively in Clojure

Hi I am a bit new to Clojure/Lisp programming but I have used recursion before in C like languages, I have written the following code to sum all numbers that can be divided by three between 1 to 100. 

(defn is_div_by_3[number]
  (if( = 0 (mod number 3))
    true false) 
 )

(defn sum_of_mult3[step,sum]
  (if (= step 100)
    sum
    )
  (if (is_div_by_3 step)
    (sum_of_mult3 (+ step 1 ) (+ sum step)) 
    )
  )

My thought was to end the recursion when step reaches sum, then I would have all the multiples I need in the sum variable that I return, but my REPL seems to returning nil for both variables what might be wrong here?

Upvotes: 2

Views: 748

Answers (3)

Diego Basch
Diego Basch

Reputation: 13079

In addition to Rodrigo's answer, here's the first way I thought of solving the problem:

(defn sum-of-mult3 [n]
  (->> n
       range
       (take-nth 3)
       (apply +)))

This should be self-explanatory. Here's a more "mathematical" way without using sequences, taking into account that the sum of all numbers up to N inclusive is (N * (N + 1)) / 2.

(defn sum-of-mult3* [n]
  (let [x (quot (dec n) 3)]
    (* 3 x (inc x) 1/2)))

Like Rodrigo said, recursion is not the right tool for this task.

Upvotes: 0

Rodrigo Taboada
Rodrigo Taboada

Reputation: 2727

if is an expression not a statement. The result of the if is always one of the branches. In fact Clojure doesn't have statements has stated here:

Clojure programs are composed of expressions. Every form not handled specially by a special form or macro is considered by the compiler to be an expression, which is evaluated to yield a value. There are no declarations or statements, although sometimes expressions may be evaluated for their side-effects and their values ignored.

There is a nice online (and free) book for beginners: http://www.braveclojure.com

Other thing, the parentheses in Lisps are not equivalent to curly braces in the C-family languages. For example, I would write your is_div_by_3 function as: 

(defn div-by-3? [number]
  (zero? (mod number 3)))

I would also use a more idiomatic approach for the sum_of_mult3 function:

(defn sum-of-mult-3 [max] 
  (->> (range 1 (inc max))
       (filter div-by-3?)
       (apply +)))

I think that this code is much more expressive in its intention then the recursive version. The only trick thing is the ->> thread last macro. Take a look at this answer for an explanation of the thread last macro.

Upvotes: 4

noisesmith
noisesmith

Reputation: 20194

There are a few issues with this code.

1) Your first if in sum_of_mult3 is a noop. Nothing it returns can effect the execution of the function.

2) the second if in sum_of_mult3 has only one condition, a direct recursion if the step is a multiple of 3. For most numbers the first branch will not be taken. The second branch is simply an implicit nil. Which your function is guaranteed to return, regardless of input (even if the first arg provided is a multiple of three, the next recurred value will not be).

3) when possible use recur instead of a self call, self calls consume the stack, recur compiles into a simple loop which does not consume stack.

Finally, some style issues:

1) always put closing parens on the same line with the block they are closing. This makes Lisp style code much more readable, and if nothing else most of us also read Algol style code, and putting the parens in the right place reminds us which kind of language we are reading.

2) (if (= 0 (mod number 3)) true false) is the same as (= 0 (mod number 3) which in turn is identical to (zero? (mod number 3))

3) use (inc x) instead of (+ x 1)

4) for more than two potential actions, use case, cond, or condp

(defn sum-of-mult3
  [step sum]
  (cond (= step 100) sum
        (zero? (mod step 3)) (recur (inc step) (+ sum step))
        :else (recur (inc step) sum))

Upvotes: 2

Related Questions