-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
IN(s) #1438
Comments
@pkoppstein What part of code are you talking about? |
For the record, the current definitions of
I would also propose simplifying the current definition of IN/2 to:
|
You are right to consider the implementation of IN unpleasant, but I think your
This outputs My solution in the next post... |
I have my own versions for some jq builtins, more elegant but only a bit faster (around a 10% in this case). For example, using my version for
I like your
|
@fadado - Thanks for pointing out the error in my original proposal. The original post has been corrected. Please note that there are already definitions of all/2 and any/2 in builtin.jq. (They were written before isempty/0 was included.) Are you proposing that they be changed? My timings for your implementation of IN/1 (using your revised any/2) indicate it is significantly slower than the current definition in builtin.jq. E.g. for N=10000, the comparable u+s for the range-based test using your IN/1 is 37.2s |
If the actual builtin is faster, ok! But compare the "style":
vs
I work as a teacher and I'm always searching for beauty in the code... |
@fadado - My comment about speed was confined to Now that I've compared some timings of your |
Outdated issue; def IN(s): any(s == .; .);
def IN(src; s): any(src == s; .); Closing... |
[EDIT: The original post proposed an incorrect implementation of IN. It has been corrected.]
I believe the following establishes that the current implementation of IN(s) is both needlessly complex and slow.
Unless I'm missing something, here is an alternative (and much simpler) implementation:
And here is the test I ran (for various values of N):
Results:
Discussion:
One would expect the current implementation to be unnecessarily slow because it uses
reduce
, and therefore does no short-circuiting.The text was updated successfully, but these errors were encountered: