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

Do not use std::to_string #51

Closed
krisztiantobias opened this issue Mar 26, 2015 · 4 comments
Closed

Do not use std::to_string #51

krisztiantobias opened this issue Mar 26, 2015 · 4 comments
Assignees

Comments

@krisztiantobias
Copy link

This is simply wrong:

    case (value_t::number_float):
    {
        return std::to_string(m_value.number_float);
    }

The to_string use std::locale what can change the point to comma (happened with us, because in QT the QApplication create change this) what is not good for json. You have to use always point instead of comma (json rpc standard)

@nlohmann
Copy link
Owner

Thanks for the hint. I'll check.

@nlohmann
Copy link
Owner

Hi @krisztiantobias,

the current code does not use std::to_string to create a serialization of floating-point numbers any more, but the following snippet (see https://github.com/nlohmann/json/blob/master/src/json.hpp.re2c#L2016):

case (value_t::number_float):
{
    // 15 digits of precision allows round-trip IEEE 754
    // string->double->string
    const auto sz = static_cast<unsigned int>(std::snprintf(nullptr, 0, "%.15g", m_value.number_float));
    std::vector<char> buf(sz + 1);
    std::snprintf(&buf[0], buf.size(), "%.15g", m_value.number_float);
    return string_t(buf.data());
}

Do you see the same issue there? If so, wouldn't prepending

std::locale::global(std::locale::classic());

fix the issue?

@nlohmann nlohmann self-assigned this Mar 26, 2015
@krisztiantobias
Copy link
Author

It's fix that, but very ugly I think. In our code I fix that with this:

Before dump:

template<typename CharT>
class DecimalSeparator : public std::numpunct<CharT>
{
public:
    DecimalSeparator(CharT Separator)
    : m_Separator(Separator)
    {}

protected:
    CharT do_decimal_point()const
    {
        return m_Separator;
    }

private:
    CharT m_Separator;
};

And in dump:

case (value_t::number_float):
{
    std::stringstream ss;
    ss.imbue(std::locale(std::locale(), new DecimalSeparator<char>('.')));
    ss << m_value.number_float;
    return ss.str();
}

@nlohmann
Copy link
Owner

In fact, the above code already fixed another issue (#37) and stems from #38. Therefore, I'd like to stick with it rather than changing it to to_string().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants