ibireme / yyjson

The fastest JSON library in C
https://ibireme.github.io/yyjson/doc/doxygen/html/
MIT License
3.04k stars 262 forks source link

Consider returning NaN from yyjson_get_num #109

Closed pavelxdd closed 1 year ago

pavelxdd commented 1 year ago

Hello.

I think it might be more reasonable to return NAN from yyajson_get_num here: https://github.com/ibireme/yyjson/blob/82b30d913c084aab2c7fe1dad91d2f0bbcada83a/src/yyjson.h#L3660 and here: https://github.com/ibireme/yyjson/blob/82b30d913c084aab2c7fe1dad91d2f0bbcada83a/src/yyjson.h#L3919 (Also note the double space here ^)

The only way these functions can fail (return 0.0 currently) is when the value is not a number (literally): https://github.com/ibireme/yyjson/blob/82b30d913c084aab2c7fe1dad91d2f0bbcada83a/src/yyjson.h#L1409

ibireme commented 1 year ago

Thanks for pointing out, I will remove the extra space later.

yyjson_get_num returns NaN is more reasonable indeed. But here it returns 0 just to be consistent with other functions like yyjson_get_int/yyjson_get_real/…

pavelxdd commented 1 year ago

That's a good point. Howewer, as I see it, yyjson_get_num is a high-level function over the yyjson_get_int/yyjson_get_real/… and does the conversions and returns only doubles, and NaN is perfect fit to indicate something is not a number in a world of doubles.

It's not a big deal anyway, considering that the type of value can always be checked for number manually prior to calling yyjson_get_num.

ibireme commented 1 year ago

I wanted yyjson_get_xxx functions to return a default value (usually 0/NULL) on error. I get your point, but I’m not sure what most C developers would like from these functions. I checked some popular C JSON libraries and saw that cJSON returns NaN, while json-c, jansson, and parson all return 0. So I think returning 0 might be a better choice as it seems more common.