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

Fix #295 | String Trim Trailing Zeros bool variable #296

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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: 9 additions & 1 deletion decimal.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ var DivisionPrecision = 16
// silently lose precision.
var MarshalJSONWithoutQuotes = false

// StringTrimTrailingZeros should be set to false if you want the decimal stringify without zeros trailing.
// By default, when decimal is output as a string (for example, in JSON), zeros are truncated from it (2.00 -> 2, 3.11 -> 3.11, 13.000 -> 13).
// But this logic can be changed by this variable.
// For example, if you have numeric(10,2) values stored in your database,
// and you want your API response to always be given 2 decimal places (even 2.00, 3.00, 17.00 [not 2,3,17]),
// then this variable is a great way out.
Comment on lines +55 to +60
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// StringTrimTrailingZeros should be set to false if you want the decimal stringify without zeros trailing.
// By default, when decimal is output as a string (for example, in JSON), zeros are truncated from it (2.00 -> 2, 3.11 -> 3.11, 13.000 -> 13).
// But this logic can be changed by this variable.
// For example, if you have numeric(10,2) values stored in your database,
// and you want your API response to always be given 2 decimal places (even 2.00, 3.00, 17.00 [not 2,3,17]),
// then this variable is a great way out.
// TrimTrailingZeros specifies whether trailing zeroes should be trimmed from a string representation of decimal.
// If set to true, trailing zeroes will be truncated (2.00 -> 2, 3.11 -> 3.11, 13.000 -> 13),
// otherwise trailing zeroes will be preserved (2.00 -> 2.00, 3.11 -> 3.11, 13.000 -> 13.000).
// Setting this value to false can be useful for APIs where exact decimal string representation matters.

Copy link
Member

Choose a reason for hiding this comment

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

What do you think about it? A bit simplified and a bit more explicit version of your suggestion.

var StringTrimTrailingZeros = true
Copy link
Member

Choose a reason for hiding this comment

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

I think we can simply call it TrimTrailingZeros, trimming is an action widely used on strings in various languages. Or do you think it could be a bit misleading as NewFromString or RequiredFromString would not perform trimming?

Copy link
Member

Choose a reason for hiding this comment

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

I also wonder whether we should introduce another config var for specifying NewFromString behaviour. The current implementation does not trim zeroes during the initialization from string, but we saw in the past that some developers would like to see such an option.
Unfortunately, we cannot use this newly introduced variable for both of these cases as we would break backward compatibility.


// ExpMaxIterations specifies the maximum number of iterations needed to calculate
// precise natural exponent value using ExpHullAbrham method.
var ExpMaxIterations = 1000
Expand Down Expand Up @@ -1022,7 +1030,7 @@ func (d Decimal) InexactFloat64() float64 {
// -12.345
//
func (d Decimal) String() string {
return d.string(true)
return d.string(StringTrimTrailingZeros)
}

// StringFixed returns a rounded fixed-point string with places digits after
Expand Down
45 changes: 45 additions & 0 deletions decimal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3339,3 +3339,48 @@ func ExampleNewFromFloat() {
//0.123123123123123
//-10000000000000
}

func TestDecimal_String(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

Could we separate test cases for TrimTrailingZeros=true and TrimTrailingZeros=false? Thanks to that we could easily catch potential bugs and it would improve overall code maintainability.

type testData struct {
input string
expected string
}

tests := []testData{
{"1.22", "1.22"},
{"1.00", "1"},
{"153.192", "153.192"},
{"999.999", "999.999"},
{"0.0000000001", "0.0000000001"},
{"0.0000000000", "0"},
}

for _, test := range tests {
d, err := NewFromString(test.input);
if err != nil {
t.Fatal(err)
} else if d.String() != test.expected {
t.Errorf("expected %s, got %s", test.expected, d.String())
}
}

defer func() {
StringTrimTrailingZeros = true
}()

StringTrimTrailingZeros = false
tests = []testData{
{"1.00", "1.00"},
{"0.00", "0.00"},
{"129.123000", "129.123000"},
}

for _, test := range tests {
d, err := NewFromString(test.input);
if err != nil {
t.Fatal(err)
} else if d.String() != test.expected {
t.Errorf("expected %s, got %s", test.expected, d.String())
}
}
}