Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Also support maps as arguments when public APIs use trailing map args #59

Closed
richievos opened this issue Jan 11, 2014 · 4 comments
Closed

Comments

@richievos
Copy link
Contributor

There's some APIs in elastisch that support trailing named arguments. For instance native.document.search

(defn search
  "Performs a search query across one or more indexes and one or more mapping types.

   Examples:

   (require '[clojurewerkz.elastisch.native.document :as doc])
   (require '[clojurewerkz.elastisch.query :as q])

   (doc/search \"people\" \"person\" :query (q/prefix :username \"appl\"))"
  [index mapping-type & {:as options}]
  (let [ft                  (es/search (cnv/->search-request index mapping-type options))
        ^SearchResponse res (.actionGet ft)]
    (cnv/search-response->seq res)))

This is handy when I'm directly calling search, ala

(es-doc/seach "foo" "bar" :size 10)

but is a PITA when trying to interact with search from another method that takes options. For instance, if I wanted to have a search method that I want to take a query and filter option, I end up having to write awkward code like:

(ns my.custom.type.that.has.defaults.and.such ...)

(defn search [name & {:as options}]
  (es-doc/search (concat ["myindex" "mytype" :size 15]
                         (flatten (vec (select-keys options [:query :filter]))))))

Rather than start my whole rant against hating the trailing named argument pattern (versus just passing a map), I'm asking here that when elastisch supports trailing named arguments it also supports passing them in as a map.

There may be a clojure pattern that makes this easier that I'm unaware of, but on the receiving side this the following works:

(defn search
  "Performs a search query across one or more indexes and one or more mapping types.

   Examples:

   (require '[clojurewerkz.elastisch.native.document :as doc])
   (require '[clojurewerkz.elastisch.query :as q])

   (doc/search \"people\" \"person\" :query (q/prefix :username \"appl\"))"
  [index mapping-type & args]
  (let [options (if (map? args) args (apply array-map args))
        ft                  (es/search (cnv/->search-request index mapping-type options))
        ^SearchResponse res (.actionGet ft)]
    (cnv/search-response->seq res)))

I'm aware throwing that if in all over the place is annoying, but such is the world of trailing named args.

@michaelklishin
Copy link
Member

Sounds good.

@michaelklishin
Copy link
Member

Going to give this a try this w/e.

michaelklishin added a commit that referenced this issue Apr 5, 2014
michaelklishin added a commit that referenced this issue Apr 5, 2014
michaelklishin added a commit that referenced this issue Apr 5, 2014
michaelklishin added a commit that referenced this issue Apr 5, 2014
michaelklishin added a commit that referenced this issue Apr 5, 2014
michaelklishin added a commit that referenced this issue Apr 5, 2014
michaelklishin added a commit that referenced this issue Apr 5, 2014
michaelklishin added a commit that referenced this issue Apr 5, 2014
michaelklishin added a commit that referenced this issue Apr 5, 2014
@michaelklishin
Copy link
Member

Should be fixed in master, will be in 2.0.0-beta4 shipping soon.

@richievos
Copy link
Contributor Author

Thanks. This is awesome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants