6

I have a function that begins like this:

(defn data-one [suser]
    (def suser-first-name
       (select db/firstNames
            (fields :firstname)
            (where {:username suser})))
    (def suser-middle-name
        (select db/middleNames
            (fields :middlename)
            (where {:username suser})))
    (def suser-last-name
         (select db/middleNames
             (fields :lastname)
             (where {:username suser})))
    ;; And it just continues on and on...
        )

Of course, I don't like this at all. I have this pattern repeating in many areas in my code-base and I'd like to generalize this.

So, I came up with the following to start:

(def data-input {:one '[suser-first-name db/firstNames :firstname] 
                      '[suser-middle-name db/middleNames :middlename]
                      '[suser-last-name db/lastNames :lastname]})

(defpartial data-build [data-item suser]
    ;; data-item takes the arg :one in this case
     `(def (data-input data-item)
        (select (data-input data-item)
            (fields (data-input data-item))
            (where {:username suser}))))

There's really a few questions here:

-- How can I deconstruct the data-input so that it creates x functions when x is unknown, ie. that the values of :one is unknown, and that the quantities of keys in data-input is unknown.

-- I'm thinking that this is a time to create a macro, but I've never built one before, so I am hesitant on the idea.

And to give a little context, the functions must return values to be deconstructed, but I think once I get this piece solved, generalizing all of this will be doable:

(defpage "/page-one" []
    (let [suser (sesh/get :username)]       
    (data-one suser)
        [:p "Firat Name: " 
            [:i (let [[{fname :firstname}] suser-first-name]
                (format "%s" fname))]
        [:p "Middle Name: "  
            [:i (let [[{mname :emptype}] suser-middle-name]
                (format "%s" mname))]
        [:p "Last Name: " 
            [:i (let [[{lname :months}] suser-last-name]
                    (format "%s" lname))]]))
dizzystar
  • 941
  • 6
  • 17

2 Answers2

5

Some suggestions:

  • def inside a function is really nasty - you are altering the global environment, and it can cause all kinds of issues with concurrency. I would suggest storing the results in a map instead.
  • You don't need a macro here - all of the data fetches can be done relatively easily within a function

I would therefore suggest something like:

(def data-input [[:suser-first-name db/firstNames :firstname] 
                 [:suser-middle-name db/middleNames :middlename]
                 [:suser-last-name db/lastNames :lastname]])

(def data-build [data-input suser]
  (loop [output {}
         items (seq data-input)]
    (if items
      (recur
        (let [[kw db fieldname] (first items)]
          (assoc output kw (select db (fields fieldname) (where {:username suser})))) 
        (next items))
      output)))

Not tested as I don't have your database setup - but hopefully that gives you an idea of how to do this without either macros or mutable globals!

mikera
  • 101,777
  • 23
  • 241
  • 402
  • I'm creating a variation of this item. The reasoning why I needed the keys in the data-input is because I have to both insert and select the values, and I also want to route them through the pages so that the keys in data-input are actually the page-names. Working on the destructuring right now. – dizzystar Sep 30 '12 at 23:20
  • Thanks for the concurrency and global concerns as well. I originally had this stuff page-level but I moved it out because I wanted to try something and then I left it there. Not sure if I have concurrency concerns right now, so why use Clojure, right? :D – dizzystar Sep 30 '12 at 23:35
4

Nice question. First of all here's the macro that you asked for:

(defmacro defquery [fname table fields ]
  (let [arg-name (symbol 'user-name)
        fname (symbol fname)]
    `(defn ~fname [~arg-name]
       (print ~arg-name (str ~@ fields)))))

You can call it like that:

(defquery suser-first-name db/firstNames [:firstname])

or if you prefer to keep all your configurations in a map, then it will accept string as the first argument instead of a symbol:

(defquery "suser-first-name" db/firstNames [:firstname])

Now, if you don't mind me recommending another solution, I would probably chose to use a single function closed around configuration. Something like that:

(defn make-reader [query-configurations]
  (fn [query-type user-name]
    (let [{table :table field-names :fields} 
           (get query-configurations query-type)]
      (select table
             (apply fields field-names)
             (where {:username suser})))))

(def data-input {:firstname  {:table db/firstNames  :fields :firstname} 
                 :middlename {:table db/middleNames :fields :middlename}
                 :lastname   {:table db/lastNames   :fields :lastname}})

(def query-function (make-reader data-input))

;; Example of executing a query
(query-function :firstname "tom")

By the way there's another way to use Korma:

;; This creates a template select from the table
(def table-select (select* db/firstNames))

;; This creates new select query for a specific field
(def first-name-select (fields table-select :firstname))

;; Creating yet another query that filters results by :username
(defn mkselect-for-user [suser query] 
  (where query {:username suser}))

;; Running the query for username "tom"
;; I fully specified exec function name only to show where it comes from.
(korma.core/exec (mkselect-for-user "tom" first-name-select)) 

For more information I highly recommend looking at Korma sources.

Ivan Koblik
  • 4,195
  • 1
  • 28
  • 32
  • 1
    This is a good answer. I don't want to bind up everything in Korma right now because I want to be able to make changes as easily as possible. It's a fairly interesting UI I'm creating and the database is subject to change. The current design is the best I was able to come up with when balancing flexibility and elegance, though I'm sure there are plenty of other options and arguments. This is a self-project, so I have to consider the person that would want to maintain it. I wasn't too pleased with the idea of using a macro here, so I'm glad both of you put me in the right direction. – dizzystar Sep 30 '12 at 23:25
  • @dizzystar Sure only you can ultimately decide what's best for your project. By the way you shouldn't be very concerned with concurrency if you use macro the same way as you'd use `defn`. Instead of creating map of configurations you could just call macro multiple times with different arguments (same way as `defn`). But the common consensus is to never use macro where function is enough. – Ivan Koblik Oct 01 '12 at 08:05