Clojure : function returns nil rather an evaluated expression

Question

I have written a function that sums the members of a list. Pretty straightforward, newbie stuff, but for some reason it returns nil, and I am at a loss as to why: this should be a no-brainer. The implementation and function call follow.

The implementation:

(defn add-list-members
  ([x] (add-list-members x 0))
  ([x total]
   (if (= (count x) 2)
     (+ (first x) (first (rest x)) total))
   (if (= (count x) 1)
     (+ (first x) total))
   (if (> (count x) 2)
     (recur (rest (rest x)) (+ (+ (first x) (first (rest x))) total)))))

The call to the function:

(println "our total is: " (add-list-members '(2 23 33 14 33 134 9000 98993)))

Meanwhile, I have a debugging version with print statements added, and I have been able to verify that the implementation works as it should.

Debugging version:

(defn add-list-members-debug
  ([x] (add-list-members-debug x 0))
  ([x total]
   (println ">>> our list now: " x)
   (println ">>> our total now: " total)
   (if (= (count x) 2)
     (println "outcome 2 : " (+ (first x) (first (rest x)) total)))
   (if (= (count x) 1)
     (println "outcome 1 : " (+ (first x) total)))
   (if (> (count x) 2)
     (recur (rest (rest x)) (+ (+ (first x) (first (rest x))) total)))))

Call to debugging version:

(add-list-members-debug '(2 23 33 14 33 134 9000 98993))

The development tools I am using are Vim plugged into the Leiningen REPL. Any hints about what I'm doing wrong here would be most appreciated.

No, nil is what Clojure often returns when there's nothing else to return. — Mars
– Mars, Commented Feb 2, 2015 at 4:15
Another person using vim with Clojure! Now I know that there are at least two of us. :-) — Mars
– Mars, Commented Feb 2, 2015 at 4:27

ntalbs · Accepted Answer · 2015-02-02 04:28:37Z

6

You used if in a wrong way. Consider the following code: I intentionally simplified the code to make it clear what happens.

(defn xxx [v]
  (if (= (count v) 1) "one")
  (if (= (count v) 2) "two")
  (if (> (count v) 2) "more"))

What happens if you run (xxx [1])? Perhaps you expect the code will return "one", however, it returns nil. Why?

Function returns value of its last expression, in this case, the last line (if (> (count v) 2) "more"). If you pass [1], the first if expression returns "one", which is ignored. The second if will return nil, which is also ignored. The third and the last will return nil, which is the return value of the function.

Perhaps the simplest way to compute sum of elements is:

(defn add-elements [xs] (apply + xs))

or

(defn add-elements [xs] (reduce + xs))

Your code can be reimplemented in much simpler way:

(defn add-elements
  ([xs] (add-elements xs 0))
  ([xs total] (if (empty? xs) total
                (recur (rest xs) (+ total (first xs))))))

edited Feb 2, 2015 at 4:28

answered Feb 2, 2015 at 4:19

ntalbs

29.6k8 gold badges72 silver badges86 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

user10756 Over a year ago

Thanks much for your comments and the illustration of the simplified implementation.

Mars · Accepted Answer · 2015-02-02 14:32:01Z

2

The parentheses are wrong, or at least suboptimal, and you have no base case that returns total. (About the parentheses: Your indentation, which is correct given the parentheses, shows this: Each if is on the same level; none of them are in the "else" position.) Here's one correct version:

(defn add-list-members
  ([x] (add-list-members x 0))
  ([x total]
   (if (= (count x) 0)
     total
     (if (= (count x) 2)
       (+ (first x) (first (rest x)) total)
       (if (= (count x) 1)
         (+ (first x) total)
         (if (> (count x) 2)
           (recur (rest (rest x)) (+ (+ (first x) (first (rest x))) total))))))))

You get nil as the return value because when the last if test fails, it goes to the else clause, which in this case is an empty end of a list, i.e. nil.

However, the multiple ifs can be replaced with one cond. You can also use ffirst for (first (first, fnext instead of (first (rest, and nnext instead of (rest (rest. It would also be more idiomatic to use a predicate such as empty? instead of checking for count equal to 0; I used count for the base case simply to maintain the parallel with your existing code. The simplest version might be:

(defn add-list-members [x] (apply + x))

You could also use reduce for a very simple version. You may just have wanted to experiment with recur, though.

(Well, you could do it without using the embedded else clauses, as in your original, but I wouldn't recomment it, and you still need the empty sequence base case. When you put multiple separate expressions in an function definition, each one of them always gets executed. This is inefficient and, I feel, error prone. Using multiple, separate expressions in a function definition makes sense when you're creating side effects--e.g. adding print statements for debugging.)

edited Feb 2, 2015 at 14:32

answered Feb 2, 2015 at 4:09

Mars

8,8842 gold badges47 silver badges73 bronze badges

3 Comments

user10756 Over a year ago

OK -- understood. Thank you very much for the explanation plus pointers.

user10756 Over a year ago

Yeah, I guess I thought -- erroneously -- that the function return was implicit for the passed if tests. I really had no right to think that, but I'm susceptible to wishful thinking sometimes. Thanks, again.

amalloy Over a year ago

Note also that fnext is just second. I suppose I might use it in some contexts where ffirst is also relevant, but usually it's a pretty rare function to use.

Collectives™ on Stack Overflow

Clojure : function returns nil rather an evaluated expression

2 Answers 2

1 Comment

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related