Skip to content
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

JSON data loss resulting from toString's use of scientific notation #1863

Closed
kitschpatrol opened this issue Jun 25, 2017 · 4 comments
Closed
Labels

Comments

@kitschpatrol
Copy link
Contributor

kitschpatrol commented Jun 25, 2017

Hmm, this doesn't look right:

JsonTree testValue("bigish number", static_cast<uint32_t>(4294967295));

console() << testValue << std::endl; // { "bigish number" : 4 }

I believe this is happening because JsonTree casts the uint_32t to a double, and then sends it through toString which yields the 4.29497e+09, which is subsequently truncated to 4.

If you tweak toString's implementation the to use fixed rather than scientific notation, this is resolved.

E.g., in Utilities.h:

template<typename T>
std::string toString( const T &t ) {
  std::ostringstream ss; ss << std::fixed << t; return ss.str();
}

Which gives:

JsonTree testValue("big number", static_cast<uint32_t>(4294967295));

console() << testValue << std::endl; // {  "bigish number" : 4294967295 }

But not sure of any implications elsewhere? Better to make a special case of it in JsonTree's implementation? Any thoughts before I attempt a PR?

The duplication is unpleasant... but maybe a template specialization makes sense?

Something like the following In Utilities.h:

template<typename T>
std::string toString( const T &t ) {
  std::ostringstream ss; ss << t; return ss.str();
}
	
template<>
inline std::string toString<float>( const float &t ) {
  std::ostringstream ss; ss << std::fixed << t; return ss.str();
}
	
template<>
inline std::string toString<double>( const double &t ) {
  std::ostringstream ss; ss << std::fixed << t; return ss.str();
}

FWIW this is on a Mac, haven't duplicated on Windows yet. I'm unclear on where an instance of std::ostringstream derives its default formatting from, and whether this default is globally configurable.

@MikeGitb
Copy link
Contributor

MikeGitb commented Jul 1, 2017

Have you checked if std::to_string would work too? That would be a more lightweight solution for the two specializations.

@andrewfb
Copy link
Collaborator

andrewfb commented Jul 2, 2017

The issue appears to be due to a bug in JsonTree::init() - let me know if PR #1866 fixes it on your side.

@kitschpatrol
Copy link
Contributor Author

Thanks Mike, thanks Andrew.

Andrew, your PR works for me. I wrote some unit tests to verify. PR for the tests if you want them is in #1867.

andrewfb added a commit that referenced this issue Jul 9, 2017
Addressing #1863, JSON misrepresenting integer value
richardeakin added a commit that referenced this issue Jul 11, 2017
@richardeakin
Copy link
Collaborator

Fixed in #1866

3togo pushed a commit to 3togo/Cinder that referenced this issue Aug 12, 2019
3togo pushed a commit to 3togo/Cinder that referenced this issue Aug 12, 2019
3togo pushed a commit to 3togo/Cinder that referenced this issue Aug 12, 2019
Addressing cinder#1863, JSON misrepresenting integer value
3togo pushed a commit to 3togo/Cinder that referenced this issue Aug 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants