I encountered the StackOverflowError for the following code:
(defn recursive-reverse
([coll] (recursive-reverse [coll nil]))
([coll acc]
(if (= coll '()) acc
(recur (rest coll) (cons (first coll) acc)))))
though using loop would make it work:
(defn recursive-reverse [lst]
(loop [coll lst acc nil]
(if (= coll '()) acc
(recur (rest coll) (cons (first coll) acc)))))
What goes wrong with the prior code without loop?
Your bug is here:
([coll] (recursive-reverse [coll nil]))
You're calling recursive-reverse with one argument (a vector). This calls the same argument list of the function, so it does it recursively and creates a stack frame every time.
Change it to:
([coll] (recursive-reverse coll nil))
and you should be right.
(Also, separate issue, but I would generally do checking for nil rather than '() and using next rather than rest. I don't think it has any real advantage in terms of performance or anything, but it seems cleaner to me.)
(nil? x) could be much faster than (= x ()), because the compiler can emit just a single bytecode operation, the primitive null check that Java uses. Of course, the latter is pretty quick, but I suspect it's several times slower than the former. As it happens, this optimized nil-check is not implemented (yet?), but it's a reasonable optimization that might be made eventually.This worked for me:
(defn recursive-reverse
([coll] (recursive-reverse coll nil))
([coll acc]
(if (= coll '()) acc
(recur (rest coll) (cons (first coll) acc)))))
You passed the arguments to recursive-reverse inside a couple of unnecessary brackets, that's all.
