-
Notifications
You must be signed in to change notification settings - Fork 616
Datastore Query order should use an explicit ascending/descending parameter instead of "-" in a string. #1061
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
Comments
This seems like a pretty reasonable request, @krisnye. I think there might be a typo in your example and you were going for this..? var companiesDescending = companyQuery.order('size', false); I'm not totally sold on using a boolean, only because at first glance it's not very obvious what it does and it may end up warranting a trip to the docs. I like the idea of separating the two, however I feel that a literal object (probably overkill) or simply passing in an optional sign would be more intuitive. var companiesDescending = companyQuery.order('size', { ascending: false });
var companiesDescending = companyQuery.order('size', '-'); @stephenplusplus thoughts? |
I prefer the options object. |
Yes, I had a typo. I fixed it. My honest thoughts are that a JSON based API should have a JSON based query structure. One that is easy to read and easy to write, while being explicit. Here is the JSON query structure that I use to represent filters, limits, offsets and sorts.
I actually combine this query format with a TYPE/ID path which captures ancestor searches in order to represent all queries. Here are some examples. They are all both REST urls and concise query descriptions:
|
If I was to design the API today, I definitely would have went with that approach as well. I'm not sure why we went with the builder approach. Obviously, at the time, it was thought to be more convenient for the developer. In the end, it's really just a style choice, so to completely change to just a plain JS object now would be an unfair amount of work to put on developers without any clear rewards. |
True, but you can build such an object behind the scenes using the builder for backward compatibility and then expose the simpler interface when appropriate. Something to think about for the future. |
Currently:
This is bad. It is mixing the properties of a string based query language with a json based query API.
Instead, consider just adding a boolean ascending / descending property that defaults to ascending.
The text was updated successfully, but these errors were encountered: