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

Discrepancy between js and .net version in comparison operators #377

Closed
alexkharuk opened this issue Feb 3, 2023 · 8 comments · Fixed by #395
Closed

Discrepancy between js and .net version in comparison operators #377

alexkharuk opened this issue Feb 3, 2023 · 8 comments · Fixed by #395
Labels
bug Something isn't working pkg:logic

Comments

@alexkharuk
Copy link
Contributor

Hello! not sure is this is a bug, if not please feel free to update this issue.

Environment
Nuget Package: JsonLogic
Nuget Version: 3.3.1
OS: macOS
.Net Target: net6.0

Describe the bug
There are some discrepancy between js and .net version in comparison operators.

To Reproduce
i made some testing with https://jsonlogic.com/ and found that JS library allows such code.
comparison of the strings:
{"<" : ["abs", "123"]}, result: true
{">" : ["abs", "123"]}, result: false
and some silly comparison that always have result: false. i am not good with JS, but probably this is because in following samples it is comparing NaN with number:
{">" : ["abs", 123]} result: false
{"<" : ["abs", 123]} result: false

But .Net lib in this cases trows errors.

What you think about add ability to compare strings in to .net version to remove discrepancy?

@alexkharuk alexkharuk added the bug Something isn't working label Feb 3, 2023
@gregsdennis
Copy link
Owner

Trying to align their original implementation and this one is a continued headache. Their test suite is very basic, and they don't specify a lot of the behaviors.

Alignment is the goal.

@gregsdennis
Copy link
Owner

I really wish they had an actual test suite or a spec.

@gregsdennis
Copy link
Owner

Found an interesting one:

{"cat" : ["abc", {}]}
// returns "abc[object Object]"

I don't think that's something I want to replicate. I think I'm okay throwing an error for this.

@gregsdennis
Copy link
Owner

@alexkharuk I think I got it all. Please download and play with it. If you notice anything out of place, please let me know.

@alexkharuk
Copy link
Contributor Author

Hello @gregsdennis !
latest changes is great but i found one discrepancy:

rule:

{"<": [{ "var": "valueA" }, { "var": "valueB" }, { "var": "valueC" }]}

payload:
{ "valueA": "", "valueB": "4", "valueC": 5 }

js result: true
net result: false

@gregsdennis
Copy link
Owner

thanks. I'll add it to the list.

@gregsdennis
Copy link
Owner

gregsdennis commented Mar 8, 2023

Looks like they're converting the empty string to 0.

The < operator seems to go through this process:

  • check if all are strings; if so, compare (e.g. {"<": ["15", "4"]} returns true)
  • attempt to convert to numbers; if successful, compare (e.g. {"<": ["15", 4]} returns false)
  • convert to strings and compare

v4.0.1 incoming.

@alexkharuk
Copy link
Contributor Author

Thanks @gregsdennis. Now it works in the same way!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:logic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants