From 00fd189a8f128a81f0b8d3abdf406edbabc28854 Mon Sep 17 00:00:00 2001 From: Alex Peters Date: Thu, 18 Jul 2024 15:05:25 +0200 Subject: [PATCH] Review comments --- math/dec.go | 25 ++++++++++++++++--------- math/dec_rapid_test.go | 8 ++++---- math/dec_test.go | 28 +++++++++++++--------------- 3 files changed, 33 insertions(+), 28 deletions(-) diff --git a/math/dec.go b/math/dec.go index 6681c294c5ac..2f858169c6fd 100644 --- a/math/dec.go +++ b/math/dec.go @@ -53,7 +53,7 @@ var dec128Context = apd.Context{ // - "NaN" or "Infinity" -> ErrInvalidDec // // The internal representation is an arbitrary-precision decimal: Negative × Coeff × 10*Exponent -// The maximum exponent is 100_000 and must not exceeded. Following values would be invalid: +// The maximum exponent is 100_000 and must not be exceeded. Following values would be invalid: // 1e100001 -> ErrInvalidDec // -1e100001 -> ErrInvalidDec // 1e-100001 -> ErrInvalidDec @@ -105,9 +105,9 @@ func NewDecWithPrec(coeff int64, exp int32) Dec { // // The precision is much higher as long as the max exponent is not exceeded. If the max exponent is exceeded, an error is returned. // For example: -// - 1e100000 + -1e^-1 -// - 1e100000 + -9e900000 -// - 1e100000 + 0 +// - 1e100000 + -1e-1 +// - 1e100000 + 9e100000 +// - 1e100001 + 0 // We can see that in apd.BaseContext the max exponent is defined hence we cannot exceed. // // This function wraps any internal errors with a context-specific error message for clarity. @@ -128,9 +128,10 @@ func (x Dec) Add(y Dec) (Dec, error) { // The precision is much higher as long as the max exponent is not exceeded. If the max exponent is exceeded, an error is returned. // For example: // - 1e-100001 - 0 -// - 1e100000 - 1^1 +// - 1e100000 - 1e-1 +// - 1e100000 - -9e100000 // - 1e100001 - 1e100001 (upper limit exceeded) -// - -100_001 - -100_001 (lower liit exceeded) +// - 1e-100001 - 1e-100001 (lower limit exceeded) // We can see that in apd.BaseContext the max exponent is defined hence we cannot exceed. // // This function wraps any internal errors with a context-specific error message for clarity. @@ -148,8 +149,8 @@ func (x Dec) Sub(y Dec) (Dec, error) { // Quo performs division of x by y using the decimal128 context with 34 digits of precision. // It returns a new Dec or an error if the division is not feasible due to constraints of decimal128. // -// Within Quo rounding is performed and specifies the Rounder in the context to use during the rounding function. -// RoundHalfUp is used if empty or not present in Roundings. +// Within Quo half up rounding may be performed to match the defined precision. If this is unwanted, QuoExact +// should be used instead. // // Key error scenarios: // - Division by zero (e.g., `123 / 0` or `0 / 0`) results in ErrInvalidDec. @@ -159,6 +160,10 @@ func (x Dec) Sub(y Dec) (Dec, error) { // - `0 / 123` yields `0`. // - `123 / 123` yields `1.000000000000000000000000000000000`. // - `-123 / 123` yields `-1.000000000000000000000000000000000`. +// - `4 / 9` yields `0.4444444444444444444444444444444444`. +// - `5 / 9` yields `0.5555555555555555555555555555555556`. +// - `6 / 9` yields `0.6666666666666666666666666666666667`. +// - `1e-100000 / 10` yields error. // // This function is non-mutative and enhances error clarity with specific messages. func (x Dec) Quo(y Dec) (Dec, error) { @@ -192,7 +197,7 @@ func (x Dec) MulExact(y Dec) (Dec, error) { return z, nil } -// QuoExact performs division like Quo andadditionally checks for rounding. It returns ErrUnexpectedRounding if +// QuoExact performs division like Quo and additionally checks for rounding. It returns ErrUnexpectedRounding if // any rounding occurred during the division. If the division is exact, it returns the result without error. // // This function is particularly useful in financial calculations or other scenarios where precision is critical @@ -206,6 +211,8 @@ func (x Dec) MulExact(y Dec) (Dec, error) { // - `0 / 123` yields `0` without rounding. // - `123 / 123` yields `1.000000000000000000000000000000000` exactly. // - `-123 / 123` yields `-1.000000000000000000000000000000000` exactly. +// - `1 / 9` yields error for the precision limit +// - `1e-100000 / 10` yields error for crossing the lower exponent limit. // - Any division resulting in a non-terminating decimal under decimal128 precision constraints triggers ErrUnexpectedRounding. // // This function does not mutate any arguments and wraps any internal errors with a context-specific error message for clarity. diff --git a/math/dec_rapid_test.go b/math/dec_rapid_test.go index c4896159bfeb..a33f82a4fdde 100644 --- a/math/dec_rapid_test.go +++ b/math/dec_rapid_test.go @@ -200,7 +200,7 @@ func testInvalidNewNonNegativeDecFromString(t *rapid.T) { func(s string) bool { return !strings.HasPrefix(s, "-0") && !strings.HasPrefix(s, "-.0") }, ), ).Draw(t, "s") - _, err := NewDecFromString(s, AssertNotNegative()) + _, err := NewDecFromString(s) require.Error(t, err) } @@ -215,7 +215,7 @@ func testInvalidNewNonNegativeFixedDecFromString(t *rapid.T) { ), rapid.StringMatching(fmt.Sprintf(`\d*\.\d{%d,}`, n+1)), ).Draw(t, "s") - _, err := NewDecFromString(s, AssertNotNegative(), AssertMaxDecimals(n)) + _, err := NewDecFromString(s) require.Error(t, err) } @@ -226,7 +226,7 @@ func testInvalidNewPositiveDecFromString(t *rapid.T) { rapid.StringMatching("[[:alpha:]]+"), rapid.StringMatching(`^-\d*\.?\d+|0$`), ).Draw(t, "s") - _, err := NewDecFromString(s, AssertGreaterThanZero()) + _, err := NewDecFromString(s) require.Error(t, err) } @@ -239,7 +239,7 @@ func testInvalidNewPositiveFixedDecFromString(t *rapid.T) { rapid.StringMatching(`^-\d*\.?\d+|0$`), rapid.StringMatching(fmt.Sprintf(`\d*\.\d{%d,}`, n+1)), ).Draw(t, "s") - _, err := NewDecFromString(s, AssertGreaterThanZero(), AssertMaxDecimals(n)) + _, err := NewDecFromString(s) require.Error(t, err) } diff --git a/math/dec_test.go b/math/dec_test.go index d05afcf3e196..fb7134b06cad 100644 --- a/math/dec_test.go +++ b/math/dec_test.go @@ -13,10 +13,9 @@ import ( func TestNewDecFromString(t *testing.T) { specs := map[string]struct { - src string - constraints []SetupConstraint - exp Dec - expErr error + src string + exp Dec + expErr error }{ "simple decimal": { src: "1", @@ -239,13 +238,13 @@ func TestAdd(t *testing.T) { y: NewDecFromInt64(1), exp: must(NewDecWithPrec(1, 100_000).Add(NewDecFromInt64(1))), }, - "1e100000 + 0 -> err": { + "1e100001 + 0 -> err": { x: NewDecWithPrec(1, 100_001), y: NewDecFromInt64(0), expErr: ErrInvalidDec, }, - "-1e100000 + 0 -> err": { - x: NewDecWithPrec(-1, 100_001), + "-1e100001 + 0 -> err": { + x: NewDecWithPrec(1, 100_001), y: NewDecFromInt64(0), expErr: ErrInvalidDec, }, @@ -265,11 +264,10 @@ func TestAdd(t *testing.T) { func TestSub(t *testing.T) { specs := map[string]struct { - x Dec - y Dec - exp Dec - expErr error - constraints []SetupConstraint + x Dec + y Dec + exp Dec + expErr error }{ "0 - 0 = 0": { x: NewDecFromInt64(0), @@ -367,9 +365,9 @@ func TestSub(t *testing.T) { expErr: ErrInvalidDec, }, "1e100000 - -1 -> 100..1": { - x: NewDecWithPrec(1, 100_000), - y: NewDecFromInt64(-1), - exp: must(NewDecFromString("1" + strings.Repeat("0", 99_999) + "1")), + x: NewDecWithPrec(1, 100_000), + y: must(NewDecFromString("-9e100000")), + expErr: ErrInvalidDec, }, "1e-100000 - 0 = 1e-100000": { x: NewDecWithPrec(1, -100_000),