Skip to content

Add setting precision for json writers and also add decimal places precision type. #752

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions include/json/value.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,13 @@ enum CommentPlacement {
numberOfCommentPlacement
};

/** \brief Type of precision for formatting of real values.
*/
enum PrecisionType {
significantDigits = 0, ///< we set max number of significant digits in string
decimalPlaces ///< we set max number of digits after "." in string
};

//# ifdef JSON_USE_CPPTL
// typedef CppTL::AnyEnumerator<const char *> EnumMemberNames;
// typedef CppTL::AnyEnumerator<const Value &> EnumValues;
Expand Down Expand Up @@ -220,6 +227,9 @@ class JSON_API Value {
static const UInt64 maxUInt64;
#endif // defined(JSON_HAS_INT64)

/// Default precision for real value for string representation.
static const UInt defaultRealPrecision;

// Workaround for bug in the NVIDIAs CUDA 9.1 nvcc compiler
// when using gcc and clang backend compilers. CZString
// cannot be defined as private. See issue #486
Expand Down
7 changes: 6 additions & 1 deletion include/json/writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ class JSON_API StreamWriterBuilder : public StreamWriter::Factory {
- If true, outputs non-finite floating point values in the following way:
NaN values as "NaN", positive infinity as "Infinity", and negative infinity
as "-Infinity".
- "precision": int
- Number of precision digits for formatting of real values.
- "precisionType": "significant"(default) or "decimal"
- Type of precision for formatting of real values.

You can examine 'settings_` yourself
to see the defaults. You can also write and read them just like any
Expand Down Expand Up @@ -339,7 +343,8 @@ JSONCPP_STRING JSON_API valueToString(UInt value);
#endif // if defined(JSON_HAS_INT64)
JSONCPP_STRING JSON_API valueToString(LargestInt value);
JSONCPP_STRING JSON_API valueToString(LargestUInt value);
JSONCPP_STRING JSON_API valueToString(double value);
JSONCPP_STRING JSON_API valueToString(double value, unsigned int precision = Value::defaultRealPrecision,
PrecisionType precisionType = PrecisionType::significantDigits);
JSONCPP_STRING JSON_API valueToString(bool value);
JSONCPP_STRING JSON_API valueToQuotedString(const char* value);

Expand Down
14 changes: 14 additions & 0 deletions src/lib_json/json_tool.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,20 @@ static inline void fixNumericLocaleInput(char* begin, char* end) {
}
}

/**
* Delete zeros in the end of string, if it isn't last zero before '.' character.
*/
static inline void fixZerosInTheEnd(char* begin, char* end) {
end--;
while ((begin < end) && (*end == '0')) {
// don't delete last zero before point.
if (*(end - 1) != '.') {
*end = '\0';
}
end--;
}
}

} // namespace Json {

#endif // LIB_JSONCPP_JSON_TOOL_H_INCLUDED
2 changes: 2 additions & 0 deletions src/lib_json/json_value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ const LargestInt Value::minLargestInt = LargestInt(~(LargestUInt(-1) / 2));
const LargestInt Value::maxLargestInt = LargestInt(LargestUInt(-1) / 2);
const LargestUInt Value::maxLargestUInt = LargestUInt(-1);

const UInt Value::defaultRealPrecision = 17;

#if !defined(JSON_USE_INT64_DOUBLE_CONVERSION)
template <typename T, typename U>
static inline bool InRange(double d, T min, U max) {
Expand Down
39 changes: 32 additions & 7 deletions src/lib_json/json_writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,21 +118,29 @@ JSONCPP_STRING valueToString(UInt value) {
#endif // # if defined(JSON_HAS_INT64)

namespace {
JSONCPP_STRING valueToString(double value, bool useSpecialFloats, unsigned int precision) {
JSONCPP_STRING valueToString(double value, bool useSpecialFloats, unsigned int precision, PrecisionType precisionType) {
// Allocate a buffer that is more than large enough to store the 16 digits of
// precision requested below.
char buffer[36];
int len = -1;

char formatString[15];
snprintf(formatString, sizeof(formatString), "%%.%ug", precision);
if (precisionType == PrecisionType::significantDigits) {
snprintf(formatString, sizeof(formatString), "%%.%ug", precision);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be e instead of g?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about saving old behaviour with g when used the shortest variant for float, because maybe some guys expect old behaviour from code. Maybe it is better to add third type for e ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. Preserving the old way by default is a good idea.

I don't have an opinion on a third type.

Let's drop this latest commit. Then we can merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, i've dropped last commit. U can merge when test will be don.

} else {
snprintf(formatString, sizeof(formatString), "%%.%uf", precision);
}

// Print into the buffer. We need not request the alternative representation
// that always has a decimal point because JSON doesn't distinguish the
// concepts of reals and integers.
if (isfinite(value)) {
len = snprintf(buffer, sizeof(buffer), formatString, value);
fixNumericLocale(buffer, buffer + len);
// to delete use-less too much zeros in the end of string
if (precisionType == PrecisionType::decimalPlaces) {
fixZerosInTheEnd(buffer, buffer + len);
}

// try to ensure we preserve the fact that this was given to us as a double on input
if (!strchr(buffer, '.') && !strchr(buffer, 'e')) {
Expand All @@ -154,7 +162,9 @@ JSONCPP_STRING valueToString(double value, bool useSpecialFloats, unsigned int p
}
}

JSONCPP_STRING valueToString(double value) { return valueToString(value, false, 17); }
JSONCPP_STRING valueToString(double value, unsigned int precision, PrecisionType precisionType) {
return valueToString(value, false, precision, precisionType);
}

JSONCPP_STRING valueToString(bool value) { return value ? "true" : "false"; }

Expand Down Expand Up @@ -856,7 +866,8 @@ struct BuiltStyledStreamWriter : public StreamWriter
JSONCPP_STRING const& nullSymbol,
JSONCPP_STRING const& endingLineFeedSymbol,
bool useSpecialFloats,
unsigned int precision);
unsigned int precision,
PrecisionType precisionType);
int write(Value const& root, JSONCPP_OSTREAM* sout) JSONCPP_OVERRIDE;
private:
void writeValue(Value const& value);
Expand Down Expand Up @@ -885,6 +896,7 @@ struct BuiltStyledStreamWriter : public StreamWriter
bool indented_ : 1;
bool useSpecialFloats_ : 1;
unsigned int precision_;
PrecisionType precisionType_;
};
BuiltStyledStreamWriter::BuiltStyledStreamWriter(
JSONCPP_STRING const& indentation,
Expand All @@ -893,7 +905,8 @@ BuiltStyledStreamWriter::BuiltStyledStreamWriter(
JSONCPP_STRING const& nullSymbol,
JSONCPP_STRING const& endingLineFeedSymbol,
bool useSpecialFloats,
unsigned int precision)
unsigned int precision,
PrecisionType precisionType)
: rightMargin_(74)
, indentation_(indentation)
, cs_(cs)
Expand All @@ -904,6 +917,7 @@ BuiltStyledStreamWriter::BuiltStyledStreamWriter(
, indented_(false)
, useSpecialFloats_(useSpecialFloats)
, precision_(precision)
, precisionType_(precisionType)
{
}
int BuiltStyledStreamWriter::write(Value const& root, JSONCPP_OSTREAM* sout)
Expand Down Expand Up @@ -933,7 +947,7 @@ void BuiltStyledStreamWriter::writeValue(Value const& value) {
pushValue(valueToString(value.asLargestUInt()));
break;
case realValue:
pushValue(valueToString(value.asDouble(), useSpecialFloats_, precision_));
pushValue(valueToString(value.asDouble(), useSpecialFloats_, precision_, precisionType_));
break;
case stringValue:
{
Expand Down Expand Up @@ -1145,6 +1159,7 @@ StreamWriter* StreamWriterBuilder::newStreamWriter() const
{
JSONCPP_STRING indentation = settings_["indentation"].asString();
JSONCPP_STRING cs_str = settings_["commentStyle"].asString();
JSONCPP_STRING pt_str = settings_["precisionType"].asString();
bool eyc = settings_["enableYAMLCompatibility"].asBool();
bool dnp = settings_["dropNullPlaceholders"].asBool();
bool usf = settings_["useSpecialFloats"].asBool();
Expand All @@ -1157,6 +1172,14 @@ StreamWriter* StreamWriterBuilder::newStreamWriter() const
} else {
throwRuntimeError("commentStyle must be 'All' or 'None'");
}
PrecisionType precisionType(significantDigits);
if (pt_str == "significant") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These names seem long and ambiguous. Maybe exp and noexp are better names. Or something else?

Copy link
Contributor Author

@melonaerial melonaerial Mar 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used significant word for old-style precision formatting. see #752 (comment). Answer it please. If we delete old-style formatting - then of course it is better change names for exp and noexp.

precisionType = PrecisionType::significantDigits;
} else if (pt_str == "decimal") {
precisionType = PrecisionType::decimalPlaces;
} else {
throwRuntimeError("precisionType must be 'significant' or 'decimal'");
}
JSONCPP_STRING colonSymbol = " : ";
if (eyc) {
colonSymbol = ": ";
Expand All @@ -1171,7 +1194,7 @@ StreamWriter* StreamWriterBuilder::newStreamWriter() const
JSONCPP_STRING endingLineFeedSymbol;
return new BuiltStyledStreamWriter(
indentation, cs,
colonSymbol, nullSymbol, endingLineFeedSymbol, usf, pre);
colonSymbol, nullSymbol, endingLineFeedSymbol, usf, pre, precisionType);
}
static void getValidWriterKeys(std::set<JSONCPP_STRING>* valid_keys)
{
Expand All @@ -1182,6 +1205,7 @@ static void getValidWriterKeys(std::set<JSONCPP_STRING>* valid_keys)
valid_keys->insert("dropNullPlaceholders");
valid_keys->insert("useSpecialFloats");
valid_keys->insert("precision");
valid_keys->insert("precisionType");
}
bool StreamWriterBuilder::validate(Json::Value* invalid) const
{
Expand Down Expand Up @@ -1214,6 +1238,7 @@ void StreamWriterBuilder::setDefaults(Json::Value* settings)
(*settings)["dropNullPlaceholders"] = false;
(*settings)["useSpecialFloats"] = false;
(*settings)["precision"] = 17;
(*settings)["precisionType"] = "significant";
//! [StreamWriterBuilderDefaults]
}

Expand Down
21 changes: 21 additions & 0 deletions src/test_lib_json/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1757,6 +1757,27 @@ JSONTEST_FIXTURE(ValueTest, precision) {
expected = "0.25634569487374054";
result = Json::writeString(b, v);
JSONTEST_ASSERT_STRING_EQUAL(expected, result);

b.settings_["precision"] = 5;
b.settings_["precisionType"] = "decimal";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe floatPrecision and floatFormat?

Copy link
Contributor Author

@melonaerial melonaerial Mar 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, but we broke compability for guys who alreafy using "precision" option for their programs, but I don't think there are much of such people.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you're right. I forgot about that.

Yes, we need to keep the word "precision". I guess "precisionType" is fine.

v = 0.256345694873740545068;
expected = "0.25635";
result = Json::writeString(b, v);
JSONTEST_ASSERT_STRING_EQUAL(expected, result);

b.settings_["precision"] = 1;
b.settings_["precisionType"] = "decimal";
v = 0.256345694873740545068;
expected = "0.3";
result = Json::writeString(b, v);
JSONTEST_ASSERT_STRING_EQUAL(expected, result);

b.settings_["precision"] = 10;
b.settings_["precisionType"] = "decimal";
v = 0.23300000;
expected = "0.233";
result = Json::writeString(b, v);
JSONTEST_ASSERT_STRING_EQUAL(expected, result);
}

struct WriterTest : JsonTest::TestCase {};
Expand Down