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.

Sign up for GitHub

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

Jump to bottom

Make Display for Decimal more human-friendly #477

Merged
merged 5 commits into from
Mar 11, 2023
Merged

Make Display for Decimal more human-friendly #477

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e4c3d5d
Make Display for Decimal more human-friendly
jobarr-amzn Mar 10, 2023
de86c80
Update src/types/decimal.rs
jobarr-amzn Mar 10, 2023
bb418a5
Update src/types/decimal.rs
jobarr-amzn Mar 10, 2023
bd82ad8
PR feedback and skip rustfmt
jobarr-amzn Mar 10, 2023
9925c62
WIDE_NUMBER as usize
jobarr-amzn Mar 10, 2023
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
46 changes: 21 additions & 25 deletions src/types/decimal.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,41 +295,37 @@ impl TryFrom<f64> for Decimal {
}

impl Display for Decimal {
#[rustfmt::skip] // https://github.com/rust-lang/rustfmt/issues/3255
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
// Inspired by the formatting conventions of Java's BigDecimal.toString()
const WIDE_NUMBER: i64 = 6; // if you think about it, six is a lot 🙃
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be a usize, which would allow you to skip:

  • casting len as i64 down on line 308
  • casting WIDE_NUMBER as usize on line 314

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah WIDE_NUMBER should be a usize.

  • If I don't cast len as i64 on line 308 thendot_index gets inferred as usize, but that's not right because dot_index should be able to be negative.
  • As far as I understand it, I either have to cast WIDE_NUMBER as usize on line 314 to agree with len or cast WIDE_NUMBER as i64 on line 324 for easiest comparison with the negative-valued dot_index there. The latter is the right call because WIDE_NUMBER should be a usize.

Unless I'm missing something, which is probable :) I'd also happily take feedback or second opinions on the WIDE_NUMBER concept- whether we ought to have it at all, whether it ought to be applied "evenly" on both whole and fractional numbers, and how large it should be.


let digits = &*self.coefficient.magnitude.to_string();
let len = digits.len();
// The index of the decimal, relative to the magnitude representation
// 0123
// Given ABCDd-2, the decimal gets inserted at position 2.
let i_d = len as i64 + self.exponent;
// The index of the decimal point, relative to the magnitude representation
// 0123 01234
// Given ABCDd-2, the decimal gets inserted at position 2, yielding AB.CD
let dot_index = len as i64 + self.exponent;

if self.coefficient.sign == Sign::Negative {
write!(f, "-").unwrap();
};

if self.exponent == 0 && len > 6 {
// e.g. A.BCDEFGd6
write!(f, "{}.{}d{}", &digits[0..1], &digits[1..len], (i_d - 1))
} else if self.exponent == 0 {
// e.g. ABC.
if self.exponent == 0 && len > WIDE_NUMBER as usize { // e.g. A.BCDEFGd6
write!(f, "{}.{}d{}", &digits[0..1], &digits[1..len], (dot_index - 1))
} else if self.exponent == 0 { // e.g. ABC.
write!(f, "{}.", &digits)
} else if self.exponent >= 0 {
// e.g. ABCd1
// let zeroes = "0".repeat(self.exponent as usize);
} else if self.exponent >= 0 { // e.g. ABCd1
write!(f, "{}d{}", &digits, self.exponent)
} else {
// exponent < 0, there is a fractional component
if i_d > 0 {
// e.g. A.BC or AB.C
let ui_d = i_d as usize;
write!(f, "{}.{}", &digits[0..ui_d], &digits[ui_d..len])
} else if i_d > -6 {
// e.g. 0.ABC or 0.000ABC
let zeroes = "0".repeat(i_d.abs() as usize);
write!(f, "0.{}{}", &*zeroes, &digits)
} else {
// e.g. A.BCd-12
write!(f, "{}.{}d{}", &digits[0..1], &digits[1..len], (i_d - 1))
} else { // exponent < 0, there is a fractional component
if dot_index > 0 { // e.g. A.BC or AB.C
let dot_index = dot_index as usize;
write!(f, "{}.{}", &digits[0..dot_index], &digits[dot_index..len])
} else if dot_index > -WIDE_NUMBER { // e.g. 0.ABC or 0.000ABC
let width = dot_index.unsigned_abs() as usize + len;
write!(f, "0.{digits:0>width$}", width = width, digits = digits)
} else { // e.g. A.BCd-12
write!(f, "{}.{}d{}", &digits[0..1], &digits[1..len], (dot_index - 1))
}
}
}
Expand Down