Open tshemsedinov opened 4 years ago
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?
See comment: https://github.com/metarhia/sql/pull/15#pullrequestreview-464521251 @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
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.
@lundibundi @belochub