views:

546

answers:

2

When I paste this code into a REPL, it works fine:

(use 'clojure.contrib.seq-utils)
(defn- random-letter [] (char (+ (rand-int 26) 97)))
(defn- random-digit [] (rand-int 10))
(defn- random-password
  "Returns an 8-character password consisting of letters and digits as follows: aa1aa1aa"
  []
  (let [password (interpose '((random-digit)) (repeat 3 (repeat 2 '(random-letter))))]
    (apply str (flatten (map (fn [coll] (map eval coll)) password)))))

Now, I have this code with :gen-class :implements [my.ServiceInterface] and a function prefixed with - to implement the interface. I unit-test with Maven/Groovy/TestNG. Everything works fine with several other interfaces/Clojure implementations, but in this particular case, I get this error:

java.lang.RuntimeException:
java.lang.Exception: Unable to resolve symbol: random-letter in this context (NO_SOURCE_FILE:32)

I can't figure out why. The only thing I can tell is different in this function from all the other functions, is that this is the only place where I use quoting, i.e. '((random-digit)) and '(random-letter). EDIT: also, this is the only place where I use eval.

I tried defining the functions as "non-private" (defn instead of defn-). I also tried a (declare random-digit random-letter) at the top. Neither of these solves the problem.

On a side note, if you have a suggestion for a better way to implement the random-password function, I am all ears. But I'd still like to know why I am getting this error and how to get this to work.

Many thanks in advance for your help. Clojure is awesome.

Edit: here is the complete code.

(ns fred.hp2010.service.ClojurePoolerService
  (:gen-class :implements [fred.hp2010.service.PoolerService])
  (:use [clojure.contrib.seq-utils :only (flatten)]))

(def dao (fred.hp2010.persistence.Repository/getDao))

(declare find-by is-taken random-password)

(defn -addPooler [this pooler] (. dao insert "POOLER" pooler))
(defn -getPoolers [this] (. dao list "poolers"))
(defn -isEmailTaken [this email] (is-taken {"email" email}))
(defn -isUsernameTaken [this username] (is-taken {"username" username}))
(defn -login [this email password] (. dao findSingle "POOLER" {"email" email "password" password}))

(defn -changePassword [this email new-password]
  (let [updated-pooler (assoc (into {} (find-by {"email" email})) "password" new-password)]
    (. dao update "POOLER" "POOLER_ID" updated-pooler)))

(defn -resetPassword [this email]
  (let [new-password (random-password)]
    (-changePassword this email new-password)
    new-password))

(defn- find-by [params] (. dao findSingle "POOLER" params))
(defn- is-taken [params] (not (nil? (find-by params))))

(defn- random-letter [] (char (+ (rand-int 26) 97)))
(defn- random-digit [] (rand-int 10))
(defn- random-password
  "Returns an 8-character password consisting of letters and digits as follows: aa1aa1aa"
  []
  (let [password (interpose '((random-digit)) (repeat 3 (repeat 2 '(random-letter))))]
    (apply str (flatten (map (fn [coll] (map eval coll)) password)))))
+6  A: 

I don't know why you're having problems compiling this with :gen-class but I wouldn't be surprised if eval had something to do with it. eval is usually a bad idea. One thing to try (completely untested) is to use ` (backquote) instead of ' (quote) so that your symbols are namespace-qualified. Don't know if that'd help or not.

Probably better to get rid of eval though. If you turn your random-character functions into infinite lazy seqs via repeatedly you can do it this way:

(defn- random-letter [] (repeatedly #(char (+ (rand-int 26) 97))))
(defn- random-digit  [] (repeatedly #(rand-int 10)))
(defn- random-password
  "Returns an 8-character password consisting of letters and digits as follows: aa1aa1aa"
  []
  (apply str
         (mapcat (fn [[n f]] (take n (f)))
                 [[2 random-letter]
                  [1 random-digit]
                  [2 random-letter]
                  [1 random-digit]
                  [2 random-letter]])))
Brian Carper
Thanks, <code>`<code> does fix the problem, after making the functions public with <code>defn</code>. Brian, is there anything you _don't_ know about Clojure? ;-)Your point about eval being a bad idea is well taken, though. I'm trying to figure out a more elegant solution. I do like the idea of have 3 pairs of letters and interposing the digits within them, instead of the 2-1-2-1-2 solution.
Frederic Daoud
"repeatedly" is what I was missing. How about this? <code>(apply str (take 8 (flatten (repeatedly #(list (random-letter) (random-letter) (random-digit))))))</code>
Frederic Daoud
Yeah that's another good way. It reminds me of `cycle`, I'll make an edit. Anything that works is probably fine unless you're generating a billion passwords and need it to be fast.
Brian Carper
Never mind, `cycle` doesn't work for this.
Brian Carper
+3  A: 

In the top handful of lines, I'm having a bit of trouble following the syntax. In particular, why all the quotes in line 7? Delayed evaluation of all those expressions is probably not helping you. I would guess that the quoted '(random-letter) is spoiling your fun.

You can probably write simpler code while eschewing eval. I'm going to go try it in the REPL, I hope to be back soon with an improved version.

EDIT:

OK, this works:

(apply str (interpose (random-digit) (repeat 3 (apply str (repeat 2 (random-letter))))))

...and it doesn't need anything from clojure.contrib :)

The str function will mung any arguments together into a string. If the arguments are in a list, you smuggle str inside the list by using apply.

As you said, Clojure is cool!

EDIT:

Here's a function that generates a random string of alphas and numerics in accordance with a String specification:

(apply str (map (fn [c] (if (= c \a) (random-letter) (random-digit))) "aanaanaa")))

It deviates a little bit from your spec but I think it's pretty cool.

Carl Smotricz
Thanks, but your function uses the same letter and the same digit, as in gg4gg4gg. I know my comment says aa1aa1aa, but that's to show the placement of the letters and digits. I still want them to be different :) As in xy5ma8fr.
Frederic Daoud
Ah, I was wondering if that was intentional! :) But I think I have a fix for that too. BRB.
Carl Smotricz
How about this? (apply str (take 8 (flatten (repeatedly #(list (random-letter) (random-letter) (random-digit))))))
Frederic Daoud
Well, it looks like the best of our suggestions are coalescing.
Carl Smotricz
That _is_ pretty cool :)
Frederic Daoud