I'm also kind of a Clojure newbie (I've been messing with it off and on for about two years), so take my comments with a grain of salt.
In the loop in fib
, I would separate each binding pair with commas, like this:
(loop [i n, cur 1, next 1] ; etc.
Clojure treats commas as whitespace, so you can use them anywhere you can use a space. It can make binding vectors and map literals with everything on one line a lot easier to read.
It looks like your function fibs
is just making an infinite lazy sequence of Fibonacci numbers by mapping fib
over a range. You can actually write a function to do this directly; see http://clojuredocs.org/clojure_core/clojure.core/lazy-seq (the second example). I don't know how the performance compares, but it's short and a good way to learn about making your own lazy sequences.
I think your solution function is good, but https://stackoverflow.com/questions/3153396/clojure-reduce-vs-apply says there might be a tiny performance advantage to using reduce
instead, like this:
(reduce + (filter even? (fibs-until 4000000)))
The hard-coded limit in solution
makes me uncomfortable somehow. The rest of your code is very nicely packaged out into separate, reusable functions, so it feels a little odd to see solution
not. Maybe pass the limit as an argument, and then write
(println (solution 4000000))
Otherwise your code looks good to me. It's very clean and easy to read.
You might also be interested in type hinting and the memoize
function if you're working through Project Euler; they can help improve performance, especially on mathematical code. For example, if you know the argument to fib
will always fit in a Java long (64 bits), you can write fib
like this:
(defn fib [^long n] ;etc
This gives the compiler a type hint that lets it avoid reflection, and can improve performance.
You can pass memoize
a referentially transparent function (one that always returns the same value given the same arguments), and it will return a new version of the function that caches results. For example, say you'd written a naive Fibonacci like this:
(defn fib [n]
(if (< n 2)
1
(+ (fib (- n 1)) (fib (- n 2)))))
Then you could send that function to memoize
, and you'd get a new version that would save its previous results. Since that naive function keeps recalculating the same cases, you'd get a performance boost by turning those repeat calls into map lookups.
I hope some of my comments were helpful, and have fun with Clojure; it rekindled my interest in programming.