SetString("3,60") returns a NaN instead of an error

[bojanz]

I've converted bojanz/currency from cockroachdb/apd to this package, and the tests are green:
bojanz/currency@2a46a5c

However, I was surprised by one difference in behavior between the two packages:

result, ok := new(decimal.Big).SetString(n)

If I pass "3,60" to SetString(), which is a string with an invalid decimal separator, instead of getting nil, false I get NaN, true.

This forces me to check for NaN every time:

if !ok || result.IsNaN(0) {
  return Amount{}, InvalidNumberError{"Amount.Mul", n}
}

Is this intentional, or a bug?

[ericlagergren]

Interesting! I assume NaN is incorrect, but I’ll need to double check the GDA spec first.

[ericlagergren]

The GDA spec says:

The result of attempting to convert a string which does not have the syntax of a numeric string is [0,qNaN].

http://speleotrove.com/decimal/daconvs.html#reftonum

Admittedly that’s unfortunate. Returning false sounds like it should be the correct behavior, even if it returns (NaN, false). I’ll see what I can do.

[Trial97]

Hi @ericlagergren,

Is there any news regarding this?
Also is the NaN check intended after SetString to see if the decimal.Big was set correctly?

Thanks,
Trial97

[ericlagergren]

The NaN check is to see if the result is NaN, which is a valid GDA decimal.

But like I mentioned before, the library should be updated so that

SetString("invalid") // NaN, false
SetString("NaN") // NaN, true

Ultimately, though, you'd still have to check if the result is NaN. A user could pass "NaN" instead of "3,60".

[ericlagergren]

You'll want to check the conditions: #166 (comment)

[gedw99]

@bojanz just adding myself here , after our chat on Reddit .

@ericlagergren
This looks amazing 🤩