-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Used macros to disable localeconv() calls #528
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
Conversation
…() with a macro-based one (fixes #527)
I'll tell you what drives me crazy: I don't even think that a JSON library should deal with this sort of thing. None of the well-known JSON parsers do this the way I consider the right way, which is to let the user handle conversion from sub-string to value. That could be via template arguments, plugins, callbacks, wrappers, or something else. It's obviously better to me, but users have never clamored for it. Anyway, this seems like a good change to me. Hopefully this will solve the remaining problems. Thanks! |
I also found it weird that a JSON library handles issues like these. If you want to write out a number as a string instead of an actual number, I think that trying to guess the way it should appear is not something a library should do anyway. I would write out the same symbols as for a regular number in quotes and leave at that. It is not my call to make any changes to the logic though, so I did the minimal changes required to make sure the library compiles for older native Android platforms. |
That's not what I'm talking about. I don't know why anyone would convert from a number to a string, but that also doesn't hurt anything. That's only for convenience. I'm talking about serialization, which is the production of the JSON string. The JSON spec does not say anything about how numbers are represented. For example, JSON does not limit the number of digits of an integer or the size of a mantissa of a floating point number. That's why it's silly for the library to take these decisions. The parser should only parse for numeric boundaries; something else should convert to an internal representation. Writing should be even simpler. |
I actually submitted the bug about compiling for Android exactly because I needed type coercion which appeared in a later library version than was used in the project. Our server guys changed the data type of a JSON value (maybe accidentally because they use Ruby), and when an integer was parsed as a string, the whole application crashed. So I decided to update to the latest library version which had coercion. Regarding the numbers, the decimal point is definitely specified as the '.' symbol. The number of digits is indeed not specified, which means that it can be very large. In the light of that I think that it is even more important to have the ability to read the numbers as strings. Say, I have a number which can't fit even in a 64-bit variable. In that case I would read it as a string and then use a big number class which can construct itself from a string. If I couldn't read this number from JSON as a string, that would mean that it's impossible to read it at all. |
I think I misunderstood your comment before. Yes, I agree. |
You might like the gason library, which I admire for its simplicity. It produced linked lists instead of dictionaries, but it is very efficient without the low-level optimizations that you can find in rapidjson, e.g. |
It is pretty remarkable, that's true. Thanks for sharing! I wouldn't use it in our project though, type coercion isn't supported there, and while being fast is nice, correctness is much more important for us. |
Fixes issue #527. Some NDK platforms don't have locale support, so there should be a reliable way to disable the support for locales. Since it isn't possible to determine the current target platform, the best way to do this is using macros.