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

Alternative syntax for .where() #14

Open
tshemsedinov opened this issue Aug 8, 2020 · 3 comments
Open

Alternative syntax for .where() #14

tshemsedinov opened this issue Aug 8, 2020 · 3 comments

Comments

@tshemsedinov
Copy link
Member

where({
  field1: 'value', // equal
  field2: [-100, 100], // between
  field3: [100, ], // greater then
  field4: [, 100], // less then
  field5: ['>', 100], // greater then
  field6: ['<', 100], // less then
  field7: ['<>', value], // not equeal
  field8: ['>=', value], // greater then or eaual to
  field9: ['<=', value], // less then or equal to
  field10: new Set([1, 2, 3]), // in set
})

@lundibundi @belochub

@lundibundi
Copy link
Member

lundibundi commented Aug 8, 2020

Why is this needed?

We have a builder syntax which is more concise, clearer to look at and doesn't create lots of unnecessary objects for each clause (this basically creates object + case_num * Array for each such check not counting the values to check compared to simple builder one-liners calls that will most likely be always inlined)

Current API (would be) is

builder.whereEqual('field1', 'value')
  .whereBetween('field2', -100, 100)
  .whereMore('field3', 100)
  .whereLess('field4', 100)
  ...
  .whereIn('field10', [1, 2, 3])

Is this needed for some programmatic use of making batch queries, then we can come up with a more efficient syntax I think?

@tshemsedinov
Copy link
Member Author

See comment: #15 (review)
@lundibundi We need declarative syntax, maybe not exactly what this, you can propose different one but not imperative calls. For example, something like this:

where({
  field1: between(-100, 100),
  field2: greater(100),
  field3: less(100),
})

or

where({
  field1: { greater: -100, less: 100 },
  field2: { greater: 100 },
  field3: { less: 100 },
})

or

where({
  field1: { '>': -100, '<': 100 },
  field2: { '>': 100 },
  field3: { '<': 100 },
})

FYI: @belochub

@lundibundi
Copy link
Member

I don't understand why would we replace builder calls with declarative syntax, it is okay to have it in a separate method (i.e. .whereMultiple) to support batch conditions but it is not ok to replace every method with this syntax because as I mentioned above it is both less ergonomic and less efficient than simple builder calls.

I'd be fine with adding something like .whereMultiple() with the second or third example syntax you provided (the first one adds unnecessary functions that will have to be imported every time). Though the between variant of the third syntax is kind of confusing to me.

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

No branches or pull requests

2 participants