CODEWITHSUNDEEP
clojure
Bozhidar Batsov
Bozhidar Batsov

Reputation: 56595

Problem with Clojure function

everyone, I've started working yesterday on the Euler Project in Clojure and I have a problem with one of my solutions I cannot figure out.

I have this function:

(defn find-max-palindrom-in-range [beg end]
  (reduce max
          (loop [n beg result []]
            (if (>= n end)
              result
              (recur (inc n)
                     (concat result
                             (filter #(is-palindrom? %)
                                     (map #(* n %) (range beg end)))))))))

I try to run it like this:

(find-max-palindrom-in-range 100 1000)

and I get this exception:

java.lang.Integer cannot be cast to clojure.lang.IFn
  [Thrown class java.lang.ClassCastException]

which I presume means that at some place I'm trying to evaluate an Integer as a function. I however cannot find this place and what puzzles me more is that everything works if I simply evaluate it like this:

(reduce max
          (loop [n 100 result []]
            (if (>= n 1000)
              result
              (recur (inc n)
                     (concat result
                             (filter #(is-palindrom? %)
                                     (map #(* n %) (range 100 1000))))))))

(I've just stripped down the function definition and replaced the parameters with constants)

Thanks in advance for your help and sorry that I probably bother you with idiotic mistake on my part. Btw I'm using Clojure 1.1 and the newest SLIME from ELPA.

Edit: Here is the code to is-palindrom?. I've implemented it as a text property of the number, not a numeric one.

(defn is-palindrom? [n]
  (loop [num (String/valueOf n)]
    (cond (not (= (first num) (last num))) false
          (<= (.length num) 1) true
          :else (recur (.substring num 1 (dec (.length num)))))))

Upvotes: 3

Views: 874

Answers (1)

Michał Marczyk
Michał Marczyk

Reputation: 84331

The code works at my REPL (1.1). I'd suggest that you paste it back at yours and try again -- perhaps you simply mistyped something?

Having said that, you could use this as an opportunity to make the code simpler and more obviously correct. Some low-hanging fruit (don't read if you think it could take away from your Project Euler fun, though with your logic already written down I think it shouldn't):

  1. You don't need to wrap is-palindrome? in an anonymous function to pass it to filter. Just write (filter is-palindrome? ...) instead.

  2. That loop in is-palindrome? is pretty complex. Moreover, it's not particularly efficient (first and last both make a seq out of the string first, then last needs to traverse all of it). It would be simpler and faster to (require '[clojure.contrib.str-utils2 :as str]) and use (= num (str/reverse num)).

  3. Since I mentioned efficiency, using concat in this manner is a tad dangerous -- it creates a lazy seq, which might blow up if you pile up two many levels of laziness (this will not matter in the context of Euler 4, but it's good to keep it in mind). If you really need to extend vectors to the right, prefer into.

  4. To further simplify things, you could consider breaking them apart into a function to filter a given sequence so that only palindromes remain and a separate function to return all products of two three-digit numbers. The latter can be accomplished with e.g.

    (for [f (range 100 1000)
      s (range 100 1000)
      :when (<= f s)] ; avoid duplication of effort
  (* f s))

Upvotes: 6

Related Questions