From 849f553b9dd627b5ffc199e81c2202320cfa7ea4 Mon Sep 17 00:00:00 2001 From: Thiago Arruda Date: Fri, 24 Jul 2015 13:16:41 -0300 Subject: [PATCH 1/4] Add scientific notation parsing --- decimal.go | 15 +++++++++++++-- decimal_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/decimal.go b/decimal.go index 9c68532..358ea8d 100644 --- a/decimal.go +++ b/decimal.go @@ -82,12 +82,23 @@ func New(value int64, exp int32) Decimal { func NewFromString(value string) (Decimal, error) { var intString string var exp int32 + + // Check if number is using scientific notation + eIndex := strings.IndexAny(value, "Ee") + if eIndex != -1 { + expInt, err := strconv.ParseInt(value[eIndex+1:len(value)], 10, 32) + if err != nil { + return Decimal{}, fmt.Errorf("can't convert %s to decimal: exponent is not numeric", value) + } + value = value[0:eIndex] + exp = int32(expInt) + } + parts := strings.Split(value, ".") if len(parts) == 1 { // There is no decimal point, we can just parse the original string as // an int intString = value - exp = 0 } else if len(parts) == 2 { intString = parts[0] + parts[1] expInt := -len(parts[1]) @@ -95,7 +106,7 @@ func NewFromString(value string) (Decimal, error) { // NOTE(vadim): I doubt a string could realistically be this long return Decimal{}, fmt.Errorf("can't convert %s to decimal: fractional part too long", value) } - exp = int32(expInt) + exp += int32(expInt) } else { return Decimal{}, fmt.Errorf("can't convert %s to decimal: too many .s", value) } diff --git a/decimal_test.go b/decimal_test.go index 3cfa803..fa6b028 100644 --- a/decimal_test.go +++ b/decimal_test.go @@ -32,6 +32,16 @@ var testTable = map[float64]string{ .1000000000000008: "0.1000000000000008", } +var testTableScientificNotation = map[string]string{ + "1e9": "1000000000", + "2.41E-3": "0.00241", + "24.2E-4": "0.00242", + "243E-5": "0.00243", + "-1e-5": "-0.00001", + "-245E3": "-245000", + "1.2345E-1": "0.12345", +} + func init() { // add negatives for f, s := range testTable { @@ -76,6 +86,17 @@ func TestNewFromString(t *testing.T) { d.value.String(), d.exp) } } + + for e, s := range testTableScientificNotation { + d, err := NewFromString(e) + if err != nil { + t.Errorf("error while parsing %s", e) + } else if d.String() != s { + t.Errorf("expected %s, got %s (%s, %d)", + s, d.String(), + d.value.String(), d.exp) + } + } } func TestNewFromStringErrs(t *testing.T) { @@ -95,6 +116,9 @@ func TestNewFromStringErrs(t *testing.T) { ".5.2", "8..2", "8.1.", + "1e", + "1-e", + "1e9e", } for _, s := range tests { From 3e7f1c747e3d43706f8a410783bfe52f4ccc61b8 Mon Sep 17 00:00:00 2001 From: Thiago Arruda Date: Tue, 28 Jul 2015 08:08:24 -0300 Subject: [PATCH 2/4] Change exp to int64 to prevent overflow --- decimal.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/decimal.go b/decimal.go index 358ea8d..bfb7699 100644 --- a/decimal.go +++ b/decimal.go @@ -81,17 +81,17 @@ func New(value int64, exp int32) Decimal { // func NewFromString(value string) (Decimal, error) { var intString string - var exp int32 + var exp int64 // Check if number is using scientific notation eIndex := strings.IndexAny(value, "Ee") if eIndex != -1 { - expInt, err := strconv.ParseInt(value[eIndex+1:len(value)], 10, 32) + expInt, err := strconv.ParseInt(value[eIndex+1:len(value)], 10, 64) if err != nil { return Decimal{}, fmt.Errorf("can't convert %s to decimal: exponent is not numeric", value) } value = value[0:eIndex] - exp = int32(expInt) + exp = expInt } parts := strings.Split(value, ".") @@ -102,11 +102,7 @@ func NewFromString(value string) (Decimal, error) { } else if len(parts) == 2 { intString = parts[0] + parts[1] expInt := -len(parts[1]) - if expInt < math.MinInt32 { - // NOTE(vadim): I doubt a string could realistically be this long - return Decimal{}, fmt.Errorf("can't convert %s to decimal: fractional part too long", value) - } - exp += int32(expInt) + exp += int64(expInt) } else { return Decimal{}, fmt.Errorf("can't convert %s to decimal: too many .s", value) } @@ -117,9 +113,14 @@ func NewFromString(value string) (Decimal, error) { return Decimal{}, fmt.Errorf("can't convert %s to decimal", value) } + if exp < math.MinInt32 || exp > math.MaxInt32 { + // NOTE(vadim): I doubt a string could realistically be this long + return Decimal{}, fmt.Errorf("can't convert %s to decimal: fractional part too long", value) + } + return Decimal{ value: dValue, - exp: exp, + exp: int32(exp), }, nil } From 669901768e0fa6958dbe9deb147564873b013284 Mon Sep 17 00:00:00 2001 From: Thiago Arruda Date: Tue, 28 Jul 2015 08:27:48 -0300 Subject: [PATCH 3/4] Add negative numbers on init() and add more tests --- decimal_test.go | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/decimal_test.go b/decimal_test.go index fa6b028..d9839b0 100644 --- a/decimal_test.go +++ b/decimal_test.go @@ -33,13 +33,18 @@ var testTable = map[float64]string{ } var testTableScientificNotation = map[string]string{ - "1e9": "1000000000", - "2.41E-3": "0.00241", - "24.2E-4": "0.00242", - "243E-5": "0.00243", - "-1e-5": "-0.00001", - "-245E3": "-245000", - "1.2345E-1": "0.12345", + "1e9": "1000000000", + "2.41E-3": "0.00241", + "24.2E-4": "0.00242", + "243E-5": "0.00243", + "1e-5": "0.00001", + "245E3": "245000", + "1.2345E-1": "0.12345", + "0e5": "0", + "0e-5": "0", + "123.456e0": "123.456", + "123.456e2": "12345.6", + "123.456e10": "1234560000000", } func init() { @@ -49,6 +54,11 @@ func init() { testTable[-f] = "-" + s } } + for e, s := range testTableScientificNotation { + if string(e[0]) != "-" && s != "0" { + testTableScientificNotation["-"+e] = "-" + s + } + } } func TestNewFromFloat(t *testing.T) { @@ -119,6 +129,12 @@ func TestNewFromStringErrs(t *testing.T) { "1e", "1-e", "1e9e", + "1ee9", + "1ee", + "1e1.2", + "123.456e1.3", + "1e-1.2", + "123.456e-1.3", } for _, s := range tests { From 632e313f4a7c7966eb2edb9c4ca03a19386bd1a8 Mon Sep 17 00:00:00 2001 From: Thiago Arruda Date: Fri, 31 Jul 2015 13:36:54 -0300 Subject: [PATCH 4/4] Fix possible int64 overflow and remove unnecessary code --- decimal.go | 8 ++++++-- decimal_test.go | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/decimal.go b/decimal.go index bfb7699..8ae929e 100644 --- a/decimal.go +++ b/decimal.go @@ -86,11 +86,15 @@ func NewFromString(value string) (Decimal, error) { // Check if number is using scientific notation eIndex := strings.IndexAny(value, "Ee") if eIndex != -1 { - expInt, err := strconv.ParseInt(value[eIndex+1:len(value)], 10, 64) + expInt, err := strconv.ParseInt(value[eIndex+1:], 10, 32) if err != nil { + rerr := err.(*strconv.NumError) + if rerr.Err == strconv.ErrRange { + return Decimal{}, fmt.Errorf("can't convert %s to decimal: fractional part too long", value) + } return Decimal{}, fmt.Errorf("can't convert %s to decimal: exponent is not numeric", value) } - value = value[0:eIndex] + value = value[:eIndex] exp = expInt } diff --git a/decimal_test.go b/decimal_test.go index d9839b0..f8fff74 100644 --- a/decimal_test.go +++ b/decimal_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "encoding/xml" "math" + "strconv" "strings" "testing" "time" @@ -135,6 +136,7 @@ func TestNewFromStringErrs(t *testing.T) { "123.456e1.3", "1e-1.2", "123.456e-1.3", + "123.456e" + strconv.FormatInt(math.MinInt64, 10), } for _, s := range tests {