-
Notifications
You must be signed in to change notification settings - Fork 72
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
Panic when rounding long fractions #69
Comments
This patch fixes the issue - should I open a pull request? @@ -593,8 +593,8 @@ impl BigDecimal {
return self.clone();
}
- let mut number = bigint.to_i128().unwrap();
- if number < 0 {
+ let mut number = bigint.clone();
+ if number < BigInt::zero() {
number = -number;
}
for _ in 0..(need_to_round_digits - 1) {
@@ -602,7 +602,7 @@ impl BigDecimal {
}
let digit = number % 10;
- if digit <= 4 {
+ if digit <= BigInt::from(4) {
self.with_scale(round_digits)
} else if bigint.is_negative() {
self.with_scale(round_digits) - BigDecimal::new(BigInt::from(1), round_digits)
@@ -2598,6 +2598,7 @@ mod bigdecimal_tests {
("1.449999999", 1, "1.4"),
("-9999.444455556666", 10, "-9999.4444555567"),
("-12345678987654321.123456789", 8, "-12345678987654321.12345679"),
+ ("0.33333333333333333333333333333333333333333333333333333333333333333333333333333333333333", 0, "0"),
];
for &(x, digits, y) in test_cases.iter() {
let a = BigDecimal::from_str(x).unwrap(); |
I ran into this one today as well. @akubera, any thoughts on merging this? |
@jubos if you're comfortable pointing to a git repo - I've forked & fixed the issue here: https://github.com/dpbriggs/bigdecimal-rs Make sure you get the right rev, so here's the line I use in my Cargo.toml:
|
Thanks @dpbriggs, I pulled this down and worked for me. |
Any thoughts on opening a PR to get this merged? |
@kyleAsilver I made a PR here: #72 |
Too bad this library is not maintained. I may have to switch to @dpbriggs fork at some point. As a temp fix I had to create a total hacked function to help divide two BigNums /// Divide (dividend / divisor) two BigInts as BigDecimals with Rounding up to 20 decimal places
pub fn divide_bigint(dividend: BigInt, divisor: BigInt, mut round: usize) -> BigDecimal {
// This function is a hack to fix BigDecimals .round() method. Because BigDecimal crashes if the decimal is larger than a u128.
// When I divide 2 huge BigNum like:
// 26959535291011309493156476344723991336010898738574164086137773096960 / 1280876111474813957445809627925710317539675495922139136
// I get a BigDecimal 21047730572451.55635883164405741141998445934612869989080465671927896842919589096896319192707531055635
// and when that massive decimal is rounded, BigDecimal crashes. So this hack function trims that decimal by the round
// parameter + 1 then calls BigDecimal.round() to avoid the crash.
// Max round without BigDecimal erroring is ~20 places
if round > 20 { round = 20; }
// Convert BigInt into BigDecimal
let dividend = BigDecimal::new(dividend, 0); // ex: 26959535291011309493156476344723991336010898738574164086137773096960
let divisor = BigDecimal::new(divisor, 0); // ex: 1280876111474813957445809627925710317539675495922139136
// Divide the two BigDecimals
// ex: 21047730572451.55635883164405741141998445934612869989080465671927896842919589096896319192707531055635
let mut result: String = (dividend / divisor).to_string();
// Split result by decimal place
// ex: ["21047730572451", "55635883164405741141998445934612869989080465671927896842919589096896319192707531055635"]
let split : Vec<&str> = result.split(".").collect();
// If length of decimal places is > round then truncate it to (round + 1)
// ex: 21047730572451.55635883164405741142
if split.len() == 2 && split[1].len() > round {
result = BigDecimal::from_str(
// Whole number + truncated decimal places
&(split[0].to_string() + "." + &split[1][0..(round + 1)].to_string())
)
.unwrap()
.round(round as i64)
.to_string();
}
// Convert and retrun string as BigDecimal
BigDecimal::from_str(&result).unwrap()
} |
I just published @dpbriggs fix as https://crates.io/crates/bigdecimal-rs while the PR isn't accepted and merged. bigdecimal = { package = "bigdecimal-rs", version = "0.2.1", features = ["serde"] } Cheers! |
This has been fixed. My apologies about the hiatus. |
The following code:
Panics when ran:
Expected result: It rounds the number to 0.
Repo with example is here.
It looks like this line is the culprit.
The text was updated successfully, but these errors were encountered: