deitch/searchjs

Type coercion for numerical comparison

Closed this issue · 7 comments

If you want to store float values in JSON, you must store them as string. Therefore, to make my life easier, I simply store all numbers as strings in my JSON.

I have not had any issues with this until now. I cannot seem to use the numerical comparisons in the jsql.

Data:

[{"popularity":"100"}, {"popularity":"50"}, {"popularity":"75"}]
[{"price":"5.95"}, {"price":"99.95"}, {"price":"49.95"}]

JSQL:

{popularity: {gt: 50}}
{price: {gt: 50}}

Please let me know if I am missing something, I would not doubt that.

If you want to store float values in JSON, you must store them as string

I don't think that is true.

> s = require('searchjs')
> q = [{"popularity":100}, {"popularity":50.2}, {"popularity":75}]
[ { popularity: 100 }, { popularity: 50.2 }, { popularity: 75 } ]
> s.matchArray(q, {popularity: {gt: 50}})
[ { popularity: 100 }, { popularity: 50.2 }, { popularity: 75 } ]
> s.matchArray(q, {popularity: {gt: 52}})
[ { popularity: 100 }, { popularity: 75 } ]

Seems to work just fine.

Yes. You are correct. I recall the root of the issue now. When encoding and decoding JSON in PHP, floats would not properly keep their decimal places. 4.95 would turn into 4.94999999999999999. This obviously is not acceptable. The only solution that I could find was to store numbers as strings.

I submitted a PR that completely resolves this issue. Its been working flawlessly in my tests so far. It just adds a simple isNaN test.

I saw the PR. I am a bit torn about it. On the one hand, if it works around an issue with no collateral damage, then should be ok, and isNAN() does a decent job of teasing out strings-that-are-numbers from strings-that-are-not-numbers.

On the other hand, type coercion does defeat the very purpose of having, well, types. Well JS is not a strongly typed language (intentionally, if I recall correctly), it does have types that can be used.

If it were core to JS, I would agree without question (well, without questioning the fix, but would question the whole system that makes it necessary, but that would be JS's fault, not ours :-) . Since it is a problem with PHP, would a better design not be to have a translating clean-up layer?

Or, perhaps, fix the issue at its core? It appears to be a 'serialize_precision' issue. Look at the following:

Fails:

$ php -r 'ini_set('serialize_precision','1'); $price = ["price" => round("45.99", 2)]; echo json_encode($price);'
{"price":5.0e+1}

Succeeds:

$ php -r 'ini_set('serialize_precision','-1'); $price = ["price" => round("45.99", 2)]; echo json_encode($price);'

{"price":45.99}

Can you just use serialize_precision to "reset" back to pre-7.1.1 php setting?

Also check the comment I am putting in now on the PR.

Wow. You rock brother! I have never been aware of that ini setting. I will definitely be updating my app with this.

Hope it works. :-)

Sorry for the delay. It looks like that serialize_precision is working properly. You can close and my PR if you like...