search

funcool / cuerdas

String manipulation library for Clojure(Script)
https://cljdoc.org/d/funcool/cuerdas/2020.03.26-3/
BSD 2-Clause "Simplified" License
304 stars 30 forks source link

change clj long parsing logic #70

Closed danboykis closed 5 years ago

danboykis commented 5 years ago

do not coerce a long string into double which causes a loss of precision

danboykis commented 5 years ago

I got bit by this today:

For example:

(.longValue (Double/valueOf "31697972792222222")) => 31697972792222224 (Long/parseLong "31697972792222222") => 31697972792222222

danboykis commented 5 years ago

This requires a bit more thought on my part since parse-long is pretty broken for java use cases.

yogthos commented 5 years ago

I would recommend a bug fix release for this as soon as possible as it's a major bug. The current implementation does not parse long values correctly, and the users will silently get wrong data when calling parse-int.

In the long term I think the name is misleading since the function doesn't just parse integers. It would be better to rename it to parse-number. I would also suggest throwing an error in case an unparsable string is pased in instead of returning a NaN. This is an error and it would be best to fail fast instead of quietly emitting a NaN value.

niwinz commented 5 years ago

It is called parse-int with the clojure conventions. clojure treats longs and ints as the same type in most circumstances (see the integer? source as example).

You are right. I will issue a new bugfix release this afternoon.

niwinz commented 5 years ago

In any case this PR, adds a duplicate parse-int function with wrong impl. I suggest submit an other PR (i'll fix it myself if this PR is not fixed for today)

In any case, thanks to all!.

yogthos commented 5 years ago

Thanks for the quick response. Regarding integer? it's a bit more nuanced because it's restricted to whole numbers even if the maximum size of a number is a long. A decimal number is not considered an integer, e.g: (integer? 1.3) => false. So, if the function is going to be called parse-int, it should be using #"-?\d+".

danboykis commented 5 years ago

@niwinz I removed the duplicated parse-int function. I think the function is keeping with the spirit of what you tried to do in terms of using Double, but relying on BigDecimal instead.