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: add quote when marshalJSON #6

Merged
merged 1 commit into from
Oct 15, 2024
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
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ lint:

test:
# run all unit-tests, ignore fuzz tests
@go test -tags='!fuzz' -v -race -failfast -coverpkg=./... -coverprofile=coverage.out -covermode=atomic ./...
@go test -tags='!fuzz' -race -failfast -coverpkg=./... -coverprofile=coverage.out -covermode=atomic ./...
@go tool cover -html=coverage.out

fuzz:
Expand All @@ -17,7 +17,7 @@ fuzz-all:
@sh scripts/fuzz-all.sh $(fuzzTime)

bench:
@go test -bench=BenchmarkUnmarshalText -benchmem -benchmem -memprofile=mem.out -cpuprofile=cpu.out -run NONE
@go test -bench=BenchmarkMarshalJSON -benchmem -benchmem -memprofile=mem.out -cpuprofile=cpu.out -run NONE

# https://stackoverflow.com/questions/6273608/how-to-pass-argument-to-makefile-from-command-line
%:
Expand Down
9 changes: 3 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ The `Decimal` type represents a fixed-point decimal number. It consists of three
```go
// decimal value = (neg == true ? -1 : 1) * coef * 10^(-prec)
type Decimal struct {
neg bool
coef bint
neg bool
prec uint8 // 0 <= prec <= 19
}

Expand All @@ -136,14 +136,11 @@ You can notice that `coef` data type is `bint`, which is a custom data type:

```go
type bint struct {
// Indicates if the coefficient exceeds 128-bit limit
overflow bool
// For coefficients exceeding u128
bigInt *big.Int

// For coefficients less than 2^128-1
u128 u128

// For coefficients exceeding u128
bigInt *big.Int
}
```

Expand Down
2 changes: 1 addition & 1 deletion benchmarks/Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

bench:
@go test -bench BenchmarkNew -benchmem -memprofile mem.out -cpuprofile cpu.out -run NONE
@go test -bench BenchmarkMarshalJSON -benchmem -memprofile mem.out -cpuprofile cpu.out -run NONE

bench-udec:
@rm -f bench-udec.txt
Expand Down
1,402 changes: 701 additions & 701 deletions benchmarks/bench-udec.txt

Large diffs are not rendered by default.

148 changes: 74 additions & 74 deletions benchmarks/benchstat.txt

Choose a reason for hiding this comment

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

I would have committed the benchmark changes for readability purpose. It wasn't easy to review altogether

Large diffs are not rendered by default.

48 changes: 35 additions & 13 deletions codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func (d Decimal) String() string {
}

if !d.coef.overflow() {
return d.stringU128(true)
return d.stringU128(true, false)
}

return d.stringBigInt(true)
Expand All @@ -33,7 +33,7 @@ func (d Decimal) StringFixed(prec uint8) string {
d1 := d.rescale(prec)

if !d1.coef.overflow() {
return d1.stringU128(false)
return d1.stringU128(false, false)
}

return d1.stringBigInt(false)
Expand Down Expand Up @@ -101,32 +101,54 @@ var (
digitBytes = [10]byte{'0', '1', '2', '3', '4', '5', '6', '7', '8', '9'}
)

func (d Decimal) stringU128(trimTrailingZeros bool) string {
return unsafeBytesToString(d.bytesU128(trimTrailingZeros))
func (d Decimal) stringU128(trimTrailingZeros bool, withQuote bool) string {
return unsafeBytesToString(d.bytesU128(trimTrailingZeros, withQuote))
}

// bytesU128 returns the byte representation of the decimal if the coefficient is u128.
func (d Decimal) bytesU128(trimTrailingZeros bool) []byte {
func (d Decimal) bytesU128(trimTrailingZeros bool, withQuote bool) []byte {
var totalLen uint8
byteLen := maxByteMap[d.coef.u128.bitLen()]
var buf []byte

if trimTrailingZeros {
// if d.prec > byteLen, that means we need to allocate upto d.prec to cover all the zeros of the fraction part
// e.g. 0.00000123, prec = 8, byteLen = 3 --> we need to allocate 8 bytes for the fraction part
if byteLen <= d.prec {
byteLen = d.prec + 1 // 1 for zero in the whole part
}

buf = make([]byte, byteLen+2) // 1 sign + 1 dot
totalLen = byteLen + 2
} else {
// if not trimming trailing zeros, we can safely allocate 41 bytes
// 1 sign + 1 dot + len(u128) (which is max to 39 bytes)
buf = []byte("00000000000000000000000000000000000000000")
// buf = []byte("00000000000000000000000000000000000000000")

Choose a reason for hiding this comment

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

I think this line should have been removed

totalLen = 41
}

if withQuote {
// if withQuote is true, we need to add quotes at the beginning and the end
totalLen += 2
buf := make([]byte, totalLen)
n := d.fillBuffer(buf[1:len(buf)-1], trimTrailingZeros)

n += 2 // 1 for quote offset at buf[l-1], 1 for moving the index to next position
l := len(buf)
buf[l-1], buf[l-n] = '"', '"'
return buf[l-n:]
}

buf := make([]byte, totalLen)
n := d.fillBuffer(buf, trimTrailingZeros)

return buf[len(buf)-n:]
}

func (d Decimal) fillBuffer(buf []byte, trimTrailingZeros bool) int {
quo, rem := d.coef.u128.QuoRem64(pow10[d.prec].lo) // max prec is 19, so we can safely use QuoRem64

prec := d.prec
l := len(buf)
n := 0
prec := d.prec

if rem != 0 {
if trimTrailingZeros {
Expand Down Expand Up @@ -175,7 +197,7 @@ func (d Decimal) bytesU128(trimTrailingZeros bool) []byte {
buf[l-n] = '-'
}

return buf[l-n:]
return n
}

func quoRem64(u u128, v uint64) (q u128, r uint64) {
Expand All @@ -196,15 +218,15 @@ func unssafeStringToBytes(s string) []byte {

func (d Decimal) MarshalJSON() ([]byte, error) {
if !d.coef.overflow() {
return d.bytesU128(true), nil
return d.bytesU128(true, true), nil
}

return []byte(d.stringBigInt(true)), nil
return []byte(`"` + d.stringBigInt(true) + `"`), nil
}

func (d *Decimal) UnmarshalJSON(data []byte) error {
var err error
*d, err = Parse(unsafeBytesToString(data))
*d, err = parseBytes(data)
return err
}

Expand Down
2 changes: 1 addition & 1 deletion codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ func TestMarshalJSON(t *testing.T) {
testcases := []struct {
in string
}{
{"123456789.123456789"},
{"0"},
{"1"},
{"-1"},
{"123456789.123456789"},
{"-123456789.123456789"},
{"0.000000001"},
{"-0.000000001"},
Expand Down
6 changes: 3 additions & 3 deletions doc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,9 @@ func ExampleDecimal_MarshalJSON() {
fmt.Println(string(b))
fmt.Println(string(c))
// Output:
// 1.23
// -1.2345
// 1234567890123456789.1234567890123456789
// "1.23"
// "-1.2345"
// "1234567890123456789.1234567890123456789"
}

func ExampleDecimal_Neg() {
Expand Down
Loading