Discussion: optargs as a function

[apa512]

How crazy would it be to add optargs with a function like in the Java driver?

Something like

(-> (r/get-all "key1" "key2") (r/optargs {:index "yolo"}))

instead of

(r/get-all ["key1" "key2"] {:index "yolo"})

The advantages would be more maintainable query code (no more checking whether an argument is supposed to be an optarg or not) and variadic arguments for functions like get-all.

[danielcompton]

Interesting idea. I'm not sure if it's better as your optargs are separate from the rest of the query term. I have a feeling the Java driver did it this way for type safety. What would the new get-all look like?

[apa512]

I'm not convinced myself. There's probably some better way to make the query code more maintainable.

I had to add a "arity or optarg" check for r/group and there are a few more functions that must be updated in the same way. Avoiding this, or at least make the code less ugly, would be nice.

[lukaszkorecki]

How about:

(r/get-all {:keys ["key1" "key2"] :index "yolo"})

this way we can nicely parametrize the query and swich keys and index from outside, without rewriting the query structure (I realize that :keys is not the best choice here as it might clash with destructuring syntax)

[danielytics]

I understand that the goal here is to simplify the implementation so that its quality can be improved, however, from a user perspective, I much prefer the existing version over all of the others suggested:

(r/get-all ["key1" "key2"] {:index "yolo"})

There are a few reasons, I think:

1. It most closely resembles the official RQL documentation
2. I dislike splitting it into two functions because it isn't two -- its one with options. The two function approach makes it look like two operations and is confusing (especially as the queries grow)
3. Being able to pass in a vector in place of ["key1" "key2"] seems desirable. You can, of course, use apply but it seems unnecessary. Feel free to ignore this reason.
4. I don't like the idea of passing in a map always because its inconsistent with both the official RQL documentation and the rest of the clj-rethinkdb functions. If this version is considered, however, the outer map could potentially be dropped: (r/get-all :keys ["key1" "key2"] :index "yolo")

[danielcompton]

In terms of implementation details, the current way is pretty tidy. In most cases it maps extremely closely to the underlying protocol, while also being fairly idiomatic Clojure. There is the messiness involved with variadic functions that have an overlap between different args. I'd probably say it's worth biting the bullet and writing functions for them, to keep the users experience better.

It's an open question for me how/if the Clojure driver could be ported to use the Java driver, and what the Clojure query language might look like then.

[danielytics]

I'd probably say it's worth biting the bullet and writing functions for them, to keep the users experience better.

You mean r/optargs like @apa512 suggested? I personally feel that this makes for a worse user experience than currently, for the reasons I listed above. Ultimately, whichever way you guys decide is fine, but I've made my preference known ;-)

It's an open question for me how/if the Clojure driver could be ported to use the Java driver, and what the Clojure query language might look like then.

I think changing the query language would be a mistake as its basically a different library then, but that discussion needs its own issue as not to go off topic here...

[danielcompton]

You mean r/optargs like @apa512 suggested?

Nope, that wasn't clear in hindsight.

I'd probably say it's worth biting the bullet and writing functions to handle the variadic arguments by examining the arguments provided to the function. It's a bit messy, but it gives a better user experience, and is somewhat common in parts of Clojure core.

[danielytics]

That's a lot less ambiguous 👍 I agree with what you're saying (obviously). A little more effort for implementors but nicer for the users.

[apa512]

Thanks for the feedback.

Conclusion: no change is warranted.