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

refactor(math): update math docs & move test code to test file #19604

Closed
wants to merge 2 commits into from
Closed
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
13 changes: 0 additions & 13 deletions math/dec.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"math/big"
"strconv"
"strings"
"testing"
)

// LegacyDec NOTE: never use new(Dec) or else we will panic unmarshalling into the
Expand Down Expand Up @@ -924,18 +923,6 @@ func LegacyMaxDec(d1, d2 LegacyDec) LegacyDec {
return d1
}

// LegacyDecEq intended to be used with require/assert: require.True(DecEq(...))
func LegacyDecEq(t *testing.T, exp, got LegacyDec) (*testing.T, bool, string, string, string) {
Copy link
Member

Choose a reason for hiding this comment

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

Removing this is api breaking.

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'll fix it

t.Helper()
return t, exp.Equal(got), "expected:\t%v\ngot:\t\t%v", exp.String(), got.String()
}

func LegacyDecApproxEq(t *testing.T, d1, d2, tol LegacyDec) (*testing.T, bool, string, string, string) {
t.Helper()
diff := d1.Sub(d2).Abs()
return t, diff.LTE(tol), "expected |d1 - d2| <:\t%v\ngot |d1 - d2| = \t\t%v", tol.String(), diff.String()
}

// FormatDec formats a decimal (as encoded in protobuf) into a value-rendered
// string following ADR-050. This function operates with string manipulation
// (instead of manipulating the sdk.Dec object).
Expand Down
18 changes: 15 additions & 3 deletions math/dec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,39 @@ func TestDecimalTestSuite(t *testing.T) {
suite.Run(t, new(decimalTestSuite))
}

// LegacyDecEq intended to be used with require/assert: require.True(DecEq(...))
func legacyDecEq(t *testing.T, exp, got math.LegacyDec) (*testing.T, bool, string, string, string) {
t.Helper()
return t, exp.Equal(got), "expected:\t%v\ngot:\t\t%v", exp.String(), got.String()
}

func legacyDecApproxEq(t *testing.T, d1, d2, tol math.LegacyDec) (*testing.T, bool, string, string, string) {
t.Helper()
diff := d1.Sub(d2).Abs()
return t, diff.LTE(tol), "expected |d1 - d2| <:\t%v\ngot |d1 - d2| = \t\t%v", tol.String(), diff.String()
}

func TestDecApproxEq(t *testing.T) {
// d1 = 0.55, d2 = 0.6, tol = 0.1
d1 := math.LegacyNewDecWithPrec(55, 2)
d2 := math.LegacyNewDecWithPrec(6, 1)
tol := math.LegacyNewDecWithPrec(1, 1)

require.True(math.LegacyDecApproxEq(t, d1, d2, tol))
require.True(legacyDecApproxEq(t, d1, d2, tol))

// d1 = 0.55, d2 = 0.6, tol = 1E-5
d1 = math.LegacyNewDecWithPrec(55, 2)
d2 = math.LegacyNewDecWithPrec(6, 1)
tol = math.LegacyNewDecWithPrec(1, 5)

require.False(math.LegacyDecApproxEq(t, d1, d2, tol))
require.False(legacyDecApproxEq(t, d1, d2, tol))

// d1 = 0.6, d2 = 0.61, tol = 0.01
d1 = math.LegacyNewDecWithPrec(6, 1)
d2 = math.LegacyNewDecWithPrec(61, 2)
tol = math.LegacyNewDecWithPrec(1, 2)

require.True(math.LegacyDecApproxEq(t, d1, d2, tol))
require.True(legacyDecApproxEq(t, d1, d2, tol))
}

// create a decimal from a decimal string (ex. "1234.5678")
Comment on lines 24 to 62
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]

For benchmark tests, it's beneficial to include a brief comment explaining what each benchmark aims to measure. This helps maintainers and contributors understand the purpose of the benchmarks and can guide future optimizations.

+ // BenchmarkMarshalTo measures the performance of the MarshalTo function.
func BenchmarkMarshalTo(b *testing.B) {
...
}

+ // BenchmarkLegacyQuoMut measures the performance of the QuoMut function on LegacyDec.
func BenchmarkLegacyQuoMut(b *testing.B) {
...
}

Expand Down
11 changes: 2 additions & 9 deletions math/int.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"math/bits"
"strings"
"sync"
"testing"
)

// MaxBitLen defines the maximum bit length supported bit Int and Uint types.
Expand Down Expand Up @@ -333,7 +332,7 @@ func (i Int) Mul(i2 Int) (res Int) {
return x
}

// MulRaw multipies Int and int64
// MulRaw multiples Int and int64
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// MulRaw multiples Int and int64
// MulRaw multiplies Int and int64

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks👍

func (i Int) MulRaw(i2 int64) Int {
return i.Mul(NewInt(i2))
}
Expand Down Expand Up @@ -414,7 +413,7 @@ func MaxInt(i, i2 Int) Int {
return Int{max(i.BigInt(), i2.BigInt())}
}

// Human readable string
// String Human readable string
Copy link
Member

@sontrinh16 sontrinh16 Mar 1, 2024

Choose a reason for hiding this comment

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

u mean String returns human readable string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, i'll fix it

func (i Int) String() string {
return i.i.String()
}
Expand Down Expand Up @@ -521,12 +520,6 @@ func (i *Int) Size() int {
func (i Int) MarshalAmino() ([]byte, error) { return i.Marshal() }
func (i *Int) UnmarshalAmino(bz []byte) error { return i.Unmarshal(bz) }

// intended to be used with require/assert: require.True(IntEq(...))
func IntEq(t *testing.T, exp, got Int) (*testing.T, bool, string, string, string) {
t.Helper()
return t, exp.Equal(got), "expected:\t%v\ngot:\t\t%v", exp.String(), got.String()
}

func hasOnlyDigits(s string) bool {
if s == "" {
return false
Expand Down
14 changes: 7 additions & 7 deletions math/uint.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func (u Uint) BigInt() *big.Int {
return new(big.Int).Set(u.i)
}

// BigInt converts Uint to big.Int, mutative the input
// BigIntMut converts Uint to big.Int, mutative the input
func (u Uint) BigIntMut() *big.Int {
if u.IsNil() {
return nil
Expand Down Expand Up @@ -99,7 +99,7 @@ func (u Uint) LTE(u2 Uint) bool { return !u.GT(u2) }
// Add adds Uint from another
func (u Uint) Add(u2 Uint) Uint { return NewUintFromBigInt(new(big.Int).Add(u.i, u2.i)) }

// Add convert uint64 and add it to Uint
// AddUint64 convert uint64 and add it to Uint
func (u Uint) AddUint64(u2 uint64) Uint { return u.Add(NewUint(u2)) }

// Sub adds Uint from another
Expand All @@ -113,7 +113,7 @@ func (u Uint) Mul(u2 Uint) (res Uint) {
return NewUintFromBigInt(new(big.Int).Mul(u.i, u2.i))
}

// Mul multiplies two Uints
// MulUint64 multiplies two Uints
func (u Uint) MulUint64(u2 uint64) (res Uint) { return u.Mul(NewUint(u2)) }

// Quo divides Uint with Uint
Expand All @@ -139,13 +139,13 @@ func (u Uint) Decr() Uint {
return u.Sub(OneUint())
}

// Quo divides Uint with uint64
// QuoUint64 divides Uint with uint64
func (u Uint) QuoUint64(u2 uint64) Uint { return u.Quo(NewUint(u2)) }

// Return the minimum of the Uints
// MinUint Return the minimum of the Uints
func MinUint(u1, u2 Uint) Uint { return NewUintFromBigInt(min(u1.i, u2.i)) }

// Return the maximum of the Uints
// MaxUint Return the maximum of the Uints
func MaxUint(u1, u2 Uint) Uint { return NewUintFromBigInt(max(u1.i, u2.i)) }

// Human readable string
Expand Down Expand Up @@ -219,7 +219,7 @@ func (u *Uint) Size() int {
return len(bz)
}

// Override Amino binary serialization by proxying to protobuf.
// MarshalAmino Override Amino binary serialization by proxying to protobuf.
func (u Uint) MarshalAmino() ([]byte, error) { return u.Marshal() }
func (u *Uint) UnmarshalAmino(bz []byte) error { return u.Unmarshal(bz) }

Expand Down
Loading