apa512 / clj-rethinkdb

Eclipse Public License 1.0
204 stars 42 forks source link

Makes get-all api consistent with reql by accepting non coll argument. #136

Closed AfonsoTsukamoto closed 8 years ago

AfonsoTsukamoto commented 8 years ago

Reql expects an argument list for get-all, so it can either be a collection or a single value.

Clojure has no way (that I know of) to receive an undefined number of arguments as a 'middle' argument and the last argument needs to be the options so remainder won't work.

This tiny PR makes sure you can pass a single value as arg to get-all so it gets closer to reql and uses a cond to ensure that. It won't break previous implementations and for composite keys it will keep the old behaviour (nested array) since there's no unambiguous way to turn around it (again - that I know of).

(motivation: spent an hour trying to figure out what was wrong in an update - turns out, was not passing a coll but a single string - it's on me, but this stops it from happening to someone else :) )

apa512 commented 8 years ago

I'm not a fan of making the single argument a special case. It's probably better to reinforce that variable length argument list in reql means vector in Clojure so I added a warning instead (https://github.com/apa512/clj-rethinkdb/commit/3ee5796643fe73e55c517fd2dc8da17304fe6bf3).

AfonsoTsukamoto commented 8 years ago

Ok, it makes sense. I was also worried with composed indexes.

Thanks!