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

strconv: should make Decimal private? #1390

Open
liuly0322 opened this issue Dec 31, 2024 · 0 comments
Open

strconv: should make Decimal private? #1390

liuly0322 opened this issue Dec 31, 2024 · 0 comments

Comments

@liuly0322
Copy link
Contributor

i.e.

patch file
diff --git a/strconv/decimal.mbt b/strconv/decimal.mbt
index 4aa7b2f..daa8c3e 100644
--- a/strconv/decimal.mbt
+++ b/strconv/decimal.mbt
@@ -19,7 +19,7 @@
/// reference:
/// - <https://nigeltao.github.io/blog/2020/parse-number-f64-simple.html>
/// - <https://nigeltao.github.io/blog/2020/eisel-lemire.html>
-struct Decimal {
+priv struct Decimal {
  digits : Bytes
  mut digits_num : Int
  mut decimal_point : Int
@@ -41,7 +41,7 @@ let powtab = [

///|
/// Create a zero decimal.
-pub fn Decimal::new() -> Decimal {
+fn Decimal::new() -> Decimal {
  {
    digits: Bytes::new(800),
    digits_num: 0,
@@ -53,13 +53,13 @@ pub fn Decimal::new() -> Decimal {

///|
/// Create a decimal with an Int64 value.
-pub fn Decimal::from_int64(v : Int64) -> Decimal {
+fn Decimal::from_int64(v : Int64) -> Decimal {
  Decimal::new()..assign(v)
}

///|
/// Create a decimal from number string.
-pub fn Decimal::parse_decimal(str : String) -> Decimal!StrConvError {
+fn Decimal::parse_decimal(str : String) -> Decimal!StrConvError {
  let d = Decimal::new()
  if str.length() <= 0 {
    syntax_err!()
@@ -145,7 +145,7 @@ pub fn Decimal::parse_decimal(str : String) -> Decimal!StrConvError {

///|
/// Convert the decimal to Double.
-pub fn to_double(self : Decimal) -> Double!StrConvError {
+fn to_double(self : Decimal) -> Double!StrConvError {
  let mut exponent = 0
  let mut mantissa = 0L
  // check the underflow and overflow
@@ -231,7 +231,7 @@ pub fn to_double(self : Decimal) -> Double!StrConvError {
///|
/// Binary shift left (s > 0) or right (s < 0).
/// The shift count must not larger than the max_shift to avoid overflow.
-pub fn shift(self : Decimal, s : Int) -> Unit {
+fn shift(self : Decimal, s : Int) -> Unit {
  if self.digits_num == 0 {
    return
  }

Why

  1. strconv should only provide:
    Use `parse_bool`, `parse_double`, `parse_int`, and `parse_int64` convert strings to values.
  2. The current Decimal is only used for parse_double. Go also only uses it internally: https://github.com/golang/go/blob/go1.15.3/src/strconv/decimal.go#L6
  3. The current Decimal implementation is only correct for internal usage:
    let d = @strconv.Decimal::from_int64(-1)
    let e = match d.to_double?() {
      Ok(x) => x
      Err(_) => {
        println("error")
        0.0
      }
    }
    println(e)
    The above user code will print 0, as negative sign is not handled properly.

Willing to make a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant