anhero / JsonBox

This is a JSON C++ library. It can write and read JSON files with ease and speed.
MIT License
115 stars 60 forks source link

getDouble() fails if number has no decimal digits and no exponent #9

Closed rhuitl closed 9 years ago

rhuitl commented 11 years ago

When writing double to a JSON file, some numbers are exported without decimal digits (which by itself is fine and shouldn't be changed):

                    "coeffs" : [
                            -4.14925e-05,
                            -3.12482e-05,
                            1,
                            -0.00257958
                    ],

However when loading this file, getDouble() returns the third element as 0.0 because it is treated as an integer. I think getDouble() should return integer values as doubles, because there is no loss involved in the conversion and the current behavior is broken.

rhuitl commented 11 years ago

Here's a fix:

diff --git a/src/Value.cpp b/src/Value.cpp
index a542ad0..b1a9ad0 100644
--- a/src/Value.cpp
+++ b/src/Value.cpp
@@ -441,7 +441,11 @@ namespace JsonBox {
        }

        double Value::getDouble() const {
-               return (type == DOUBLE) ? (*data.doubleValue) : (EMPTY_DOUBLE);
+               switch (type) {
+               case DOUBLE:  return *data.doubleValue;
+               case INTEGER: return *data.intValue;
+               default:      return EMPTY_DOUBLE;
+               }
        }

        void Value::setDouble(double newDouble) {
juml commented 11 years ago

(Still) might be useful to fix output JSON stream generation (and put 1.0 for double, not 1 for this case)

madbranch commented 11 years ago

I'll look into it. In another project I am working on, we have a sort of layer over this library that is very similar and we've fixed the getDouble() problem. (https://github.com/twistedjoe/EntityBB/blob/develop/BaconBox/Helper/Serialization/Value.cpp#L397)

madbranch commented 11 years ago

I've added some methods to make it easier to get a Value's contents.

https://github.com/anhero/JsonBox/commit/18ddb017f8cc9a707c75879e4555df8d9439e0eb

Meekohi commented 10 years ago

These are worth pulling into master for a release! Would have saved me a lot of time and confusion.

madbranch commented 9 years ago

I am currently preparing for a new release soon.

russell-taylor commented 9 years ago

The sooner the better. We just tripped over this on another project and are pulling that commit to avoid the problem.