Clojure: nullpointerexception with recursion

Question

I am creating an overloaded function that takes a list of numbers as a parameter (as well as a list of numbers and a single number in the overloaded signature). From this list the highest value must be found and returned. I am aware of the available "sort" method which automatically sorts the list for you but I am attempting to discover the highest value using recursion. Below is the code I am working with.

32    (defn my-max
33      ([list]
34        (if (empty? list)
35          -1
36          (my-max (pop list) (first list))) ; recursive call
37        )
38      ([list high]
39        (if (empty? list) ; base case
40          high)
41        (if (> (first list) high)
42          (my-max (pop list) (first list)))
43        (my-max (pop list) high))
44      )
45    
46    (println (my-max '(1 2 3)))

As you can see, I have hard-coded a list containing 1, 2, and 3 for which 3 should be returned. when running the code I receive an error:

Exception in thread "main" java.lang.NullPointerException
    at clojure.lang.Numbers.ops(Numbers.java:942)
    at clojure.lang.Numbers.gt(Numbers.java:227)
    at user$my_max.invoke(HelloWorld.clj:41)
    at user$my_max.invoke(HelloWorld.clj:42)
    at user$my_max.invoke(HelloWorld.clj:42)
    at user$my_max.invoke(HelloWorld.clj:36)
    at user$eval2.invoke(HelloWorld.clj:46)
    at clojure.lang.Compiler.eval(Compiler.java:6619)
    at clojure.lang.Compiler.load(Compiler.java:7064)
    at clojure.lang.Compiler.loadFile(Compiler.java:7020)
    at clojure.main$load_script.invoke(main.clj:294)
    at clojure.main$script_opt.invoke(main.clj:356)
    at clojure.main$main.doInvoke(main.clj:440)
    at clojure.lang.RestFn.invoke(RestFn.java:408)
    at clojure.lang.Var.invoke(Var.java:415)
    at clojure.lang.AFn.applyToHelper(AFn.java:161)
    at clojure.lang.Var.applyTo(Var.java:532)
    at clojure.main.main(main.java:37)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:483)
    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:134)

This error occurs on the final recursive call where 3 has been "popped" from the list and is passed back to my-max as "high" the list is empty and:

(if (empty? list) ; line 39, base case

should evaluate true but is instead evaluating as false. Do you have any thoughts/solutions on this problem?

Arthur Ulfeldt's answer pointed out that there were misplaced parentheses. Diego Basch's answer clarified how that problem led to the null pointer exception. I just wanted to point out that paying attention to proper automatic indenting (which the code in your question illustrates), can make it immediately obvious that something is wrong with parentheses, since the expressions that are supposed to be else clauses are indented to exactly the same level as the initial (if's. — Mars
– Mars, Commented Jan 23, 2015 at 22:47
Since you mention sort as a way to find the maximum in a list of numbers (not really efficient), be aware of max in clojure.core. — T.Gounelle
– T.Gounelle, Commented Feb 13, 2015 at 17:56

Diego Basch · Accepted Answer · 2015-01-23 21:51:37Z

6

That statement (line 39-40) does nothing. No matter what it evaluates to (either high or nil), the value is not used. Then line 41 evaluates, and (first list) is nil if your list is empty, hence the null pointer exception.

answered Jan 23, 2015 at 21:51

Diego Basch

13.1k2 gold badges31 silver badges24 bronze badges

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

1 Comment

Tim X Over a year ago

In addition to the importance of using correct code indentation (and I'd recommend an editor which supports working with s-exp and balanced parens etc), it should also be noted that you should use recur rather than just calling your procedure recursively as Clojure does not have tail call optimisation. Recur will ensure that this function will work with even large lists where the current version will blow out the stack quite quickly

Arthur Ulfeldt · Accepted Answer · 2015-01-23 21:57:03Z

4

the < function can only compare numbers so a very straightforward approach is to use an or to ensure they are numbers. Also the second and third if statements had the closing ) in the wrong spots.

user> (defn my-max
       ([list]
         (if (empty? list)
          -1
           (my-max (pop list) (first list))))

       ([list high]
        (if (empty? list) ; base case
          high
         (if (> (or (first list) 0) (or high 0))
            (my-max (pop list) (first list))
            (my-max (pop list) high)))))
      #'user/my-max
      user> (println (my-max '(1 2 3)))
      3
      nil

edited Jan 23, 2015 at 21:57

answered Jan 23, 2015 at 21:52

Arthur Ulfeldt

91.7k30 gold badges205 silver badges285 bronze badges

Collectives™ on Stack Overflow

Clojure: nullpointerexception with recursion

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related