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

Add extra detail to JsonLogicExceptions where appropriate? #379

Closed
dcook-net opened this issue Feb 15, 2023 · 2 comments
Closed

Add extra detail to JsonLogicExceptions where appropriate? #379

dcook-net opened this issue Feb 15, 2023 · 2 comments
Labels
enhancement New feature in an existing library

Comments

@dcook-net
Copy link

dcook-net commented Feb 15, 2023

Environment

  • Nuget Package: JsonLogic
  • Nuget Version: 3.3.1

Can this library make doing something simpler?
Came across this exception today in LessThanEqualRule, and ended up spending a bit of time trying to work out which of the many comparisions where actually causing this.

I was thinking that this could have been sped up if the exception included the values numberA & numberb, as this would have helped zero in on the culprit faster:
`
private string AsString(decimal? number) => number.HasValue ? number.ToString() : "null";

if (numberA == null || numberB == null)
throw new JsonLogicException($"Cannot compare {a.JsonType()}:{AsString(numberA)} and {b.JsonType()}:{AsString(numberB)}.");
`
I realise that at least one of them will be null, but one may not.

If you are happy with that, we can raise a PR to make this change.

Regards,
Dave

@dcook-net dcook-net added the enhancement New feature in an existing library label Feb 15, 2023
@dcook-net dcook-net reopened this Feb 15, 2023
@gregsdennis
Copy link
Owner

gregsdennis commented Feb 15, 2023

I think this is in conflict with #377, which, if implemented, should remove all of those exceptions. I'm not sure. There's a lot of research that needs to go into that one to find out the behavior of the the JS implementation. Between the two, I would prefer to align the implementations.

However, if this can be a quick fix, I'd be happy to include this until that one can be completed.

@dcook-net
Copy link
Author

ah, ok...and the nulls wouldn't have a negative impact on the attempt to compare.
👍🏻

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

No branches or pull requests

2 participants