-
Notifications
You must be signed in to change notification settings - Fork 9
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
Prototyp, BigInt string parsing, added functions for parsing from string, fix panic #153
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,20 @@ func NewBigInt(n int64) BigInt { | |
return BigInt(*b) | ||
} | ||
|
||
// Decimal number string | ||
func NewBigIntFromNumberString(hs string) BigInt { | ||
return NewBigIntFromDecimalString(hs) | ||
} | ||
|
||
func NewBigIntFromBinaryString(hs string) BigInt { | ||
return NewBigIntFromString(hs, 2) | ||
} | ||
|
||
func NewBigIntFromOctalString(hs string) BigInt { | ||
return NewBigIntFromString(hs, 8) | ||
} | ||
|
||
func NewBigIntFromDecimalString(hs string) BigInt { | ||
return NewBigIntFromString(hs, 10) | ||
} | ||
|
||
|
@@ -30,11 +43,16 @@ func NewBigIntFromHexString(hs string) BigInt { | |
|
||
func NewBigIntFromString(s string, base int) BigInt { | ||
b := big.NewInt(0) | ||
if base != 0 && (2 > base || base > big.MaxBase) { | ||
return BigInt{} | ||
} | ||
|
||
b, ok := b.SetString(s, base) | ||
if !ok { | ||
n, _ := ParseNumberString(s, base) | ||
b = big.NewInt(n) | ||
} | ||
|
||
return BigInt(*b) | ||
} | ||
|
||
|
@@ -228,29 +246,58 @@ func (b *BigInt) UnmarshalBinary(buff []byte) error { | |
|
||
func ParseNumberString(s string, base int) (int64, bool) { | ||
var ns strings.Builder | ||
if base == 10 { | ||
switch base { | ||
case 2: | ||
for _, char := range s { | ||
// 0-9 || B || b | ||
if (char >= 48 && char <= 57) || char == 45 || char == 66 || char == 98 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch, here is fix: #166 |
||
ns.WriteRune(char) | ||
Comment on lines
+253
to
+254
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. btw: if we see a character that doesn't fit into this condition... shouldn't we error out instead of ignoring it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should, but it is written in a way, that it ignores other characters. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be for better. Ignoring some characters and treating the input as valid sounds like a bug to me. Also, I think we should introduce a function that returns an error. func ParseNumberString() (int64, error) {} but I guess we need to keep the existing function backward-compatible, so we'd need to come up with a different func name @pkieltyka thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we definitely need backwards compatibility, as well what would we use |
||
} | ||
} | ||
s = ns.String() | ||
s = strings.TrimPrefix(s, "0B") | ||
s = strings.TrimPrefix(s, "0b") | ||
|
||
case 8: | ||
for _, char := range s { | ||
// 0-9 || O || o | ||
if (char >= 48 && char <= 57) || char == 79 || char == 111 { | ||
ns.WriteRune(char) | ||
} | ||
} | ||
s = ns.String() | ||
s = strings.TrimPrefix(s, "0O") | ||
s = strings.TrimPrefix(s, "0o") | ||
|
||
case 10: | ||
for _, char := range s { | ||
// 0-9 | ||
if char >= 48 && char <= 57 { | ||
ns.WriteRune(char) | ||
} | ||
} | ||
s = ns.String() | ||
} else if base == 16 { | ||
|
||
case 16: | ||
for _, char := range s { | ||
if (char >= 48 && char <= 57) || (char == 120) || (char >= 65 && char <= 70) || (char >= 97 && char <= 102) { | ||
// 0-9 || A-Z || a-z || X || x | ||
if (char >= 48 && char <= 57) || (char >= 65 && char <= 70) || (char >= 97 && char <= 102) || char == 88 || char == 120 { | ||
ns.WriteRune(char) | ||
} | ||
} | ||
|
||
s = ns.String() | ||
s = strings.TrimPrefix(s, "0X") | ||
s = strings.TrimPrefix(s, "0x") | ||
} else { | ||
default: | ||
return 0, false | ||
} | ||
|
||
n, err := strconv.ParseInt(s, base, 64) | ||
if err != nil { | ||
return 0, false | ||
} | ||
|
||
return n, true | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,6 @@ package prototyp | |
|
||
import ( | ||
"encoding/json" | ||
"fmt" | ||
"math/big" | ||
"testing" | ||
|
||
|
@@ -58,6 +57,46 @@ func TestBigIntScan(t *testing.T) { | |
assert.Error(t, b.Scan("1.")) | ||
} | ||
|
||
func TestBigIntFromBaseString(t *testing.T) { | ||
testCases := []struct { | ||
fn func(string) BigInt | ||
input string | ||
expected int64 | ||
testName string | ||
}{ | ||
// Binary tests | ||
{NewBigIntFromBinaryString, "00101010", 42, "Binary 42"}, | ||
{NewBigIntFromBinaryString, "-00101010", -42, "Binary -42"}, | ||
{NewBigIntFromBinaryString, "0B00101010", 42, "Binary with 0B 42"}, | ||
{NewBigIntFromBinaryString, "0b00101010", 42, "Binary with 0b 42"}, | ||
|
||
// Octal tests | ||
{NewBigIntFromOctalString, "52", 42, "Octal 42"}, | ||
{NewBigIntFromOctalString, "-52", -42, "Octal -42"}, | ||
{NewBigIntFromOctalString, "0O52", 42, "Octal with 0O 42"}, | ||
{NewBigIntFromOctalString, "0o52", 42, "Octal with 0o 42"}, | ||
|
||
// Decimal tests | ||
{NewBigIntFromDecimalString, "42", 42, "Decimal 42"}, | ||
{NewBigIntFromDecimalString, "-42", -42, "Decimal -42"}, | ||
|
||
// Hexadecimal tests | ||
{NewBigIntFromHexString, "2A", 42, "Hex 2A"}, | ||
{NewBigIntFromHexString, "2a", 42, "Hex 2a"}, | ||
{NewBigIntFromHexString, "-2A", -42, "Hex -2A"}, | ||
{NewBigIntFromHexString, "-2a", -42, "Hex -2a"}, | ||
{NewBigIntFromHexString, "0x2a", 42, "Hex with 0x 42"}, | ||
{NewBigIntFromHexString, "0X2a", 42, "Hex with 0X 42"}, | ||
Comment on lines
+84
to
+89
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'd be nice to add some test cases with invalid inputs & test for errors |
||
} | ||
|
||
for _, tc := range testCases { | ||
t.Run(tc.testName, func(t *testing.T) { | ||
b := tc.fn(tc.input) | ||
assert.Equal(t, tc.expected, b.Int64()) | ||
}) | ||
} | ||
} | ||
|
||
func TestBigIntInvalidInput(t *testing.T) { | ||
b := NewBigIntFromNumberString("1234") | ||
assert.Equal(t, int64(1234), b.Int64()) | ||
|
@@ -69,12 +108,17 @@ func TestBigIntInvalidInput(t *testing.T) { | |
// if invalid, it will parse out the number | ||
b = NewBigIntFromHexString("/0xaBc-$$2Efg") | ||
assert.Equal(t, int64(0xaBc2Ef), b.Int64()) | ||
|
||
// should return 0, since base is not valid | ||
b = NewBigIntFromString("1234", 666) | ||
assert.Equal(t, int64(0), b.Int64()) | ||
} | ||
|
||
func TestBigIntCopy(t *testing.T) { | ||
a := ToBigInt(big.NewInt(1)) | ||
b := ToBigInt(a.Int()) | ||
a.Sub(big.NewInt(1)) | ||
fmt.Println(a.String()) | ||
fmt.Println(b.String()) | ||
|
||
assert.Equal(t, int64(0), a.Int64()) | ||
assert.Equal(t, int64(1), b.Int64()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this condition doesn't read very well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is horrible, but it the same conditon which is inside
math
package and which leads to panic. I can add a comment with link to that code.