From a062a9307535788f4637bc05c37bfb5214cfda26 Mon Sep 17 00:00:00 2001 From: Robin Leroy Date: Tue, 28 Jun 2022 05:23:34 +0200 Subject: [PATCH] Switch FixedDecimal to a trivaluate sign (#2025) --- utils/fixed_decimal/src/decimal.rs | 132 +++++++++++++++++++++++------ utils/fixed_decimal/src/lib.rs | 1 + 2 files changed, 109 insertions(+), 24 deletions(-) diff --git a/utils/fixed_decimal/src/decimal.rs b/utils/fixed_decimal/src/decimal.rs index 933b51b7467..624a5a41da9 100644 --- a/utils/fixed_decimal/src/decimal.rs +++ b/utils/fixed_decimal/src/decimal.rs @@ -23,7 +23,7 @@ const_assert!(core::mem::size_of::() >= core::mem::size_of::()); /// A struct containing decimal digits with efficient iteration and manipulation by magnitude /// (power of 10). Supports a mantissa of non-zero digits and a number of leading and trailing -/// zeros, used for formatting and plural selection. +/// zeros, as well as an optional sign; used for formatting and plural selection. /// /// # Data Types /// @@ -112,8 +112,21 @@ pub struct FixedDecimal { /// - <= magnitude lower_magnitude: i16, - /// Whether the number is negative. Negative zero is supported. - is_negative: bool, + /// The sign; note that a positive value may be represented by either + /// `Sign::Positive` (corresponding to a prefix +) or `Sign::None` + /// (corresponding to the absence of a prefix sign). + sign: Sign, +} + +/// A specification of the sign used when formatting a number. +#[derive(Copy, Clone, PartialEq, Eq, Debug)] +pub enum Sign { + /// No sign (implicitly positive, e.g., 1729). + None, + /// A negative sign, e.g., -1729. + Negative, + /// An explicit positive sign, e.g., +1729. + Positive, } impl Default for FixedDecimal { @@ -124,7 +137,7 @@ impl Default for FixedDecimal { magnitude: 0, upper_magnitude: 0, lower_magnitude: 0, - is_negative: false, + sign: Sign::None, } } } @@ -134,10 +147,14 @@ macro_rules! impl_from_signed_integer_type { impl From<$itype> for FixedDecimal { fn from(value: $itype) -> Self { let int_iterator: IntIterator<$utype> = value.into(); - let is_negative = int_iterator.is_negative; + let sign = if int_iterator.is_negative { + Sign::Negative + } else { + Sign::None + }; let mut result = Self::from_ascending(int_iterator) .expect("All built-in integer types should fit"); - result.is_negative = is_negative; + result.sign = sign; result } } @@ -407,6 +424,7 @@ impl FixedDecimal { /// Change the value from negative to positive or from positive to negative, modifying self. /// Negative zero is supported. + /// Negating a number with negative sign results in one with no sign (rather than an explicit positive sign). /// /// # Examples /// @@ -432,7 +450,11 @@ impl FixedDecimal { /// assert_ne!(zero, negative_zero); /// ``` pub fn negate(&mut self) { - self.is_negative = !self.is_negative; + self.sign = if self.sign == Sign::Negative { + Sign::None + } else { + Sign::Negative + }; } /// Change the value from negative to positive or from positive to negative, consuming self @@ -450,6 +472,47 @@ impl FixedDecimal { self } + /// Change the sign to the one given. + /// + /// # Examples + /// + /// ``` + /// use fixed_decimal::FixedDecimal; + /// use fixed_decimal::Sign; + /// + /// let mut dec = FixedDecimal::from(1729); + /// assert_eq!("1729", dec.to_string()); + /// + /// dec.set_sign(Sign::Negative); + /// assert_eq!("-1729", dec.to_string()); + /// + /// dec.set_sign(Sign::Positive); + /// assert_eq!("+1729", dec.to_string()); + /// + /// dec.set_sign(Sign::None); + /// assert_eq!("1729", dec.to_string()); + /// ``` + pub fn set_sign(&mut self, sign: Sign) { + self.sign = sign; + } + + /// Change the sign to the one given, consuming self and returning a new object. + /// + /// # Examples + /// + /// ``` + /// use fixed_decimal::FixedDecimal; + /// use fixed_decimal::Sign; + /// + /// assert_eq!("+1729", FixedDecimal::from(1729).with_sign(Sign::Positive).to_string()); + /// assert_eq!("1729", FixedDecimal::from(-1729).with_sign(Sign::None).to_string()); + /// assert_eq!("-1729", FixedDecimal::from(1729).with_sign(Sign::Negative).to_string()); + /// ``` + pub fn with_sign(mut self, sign: Sign) -> Self { + self.set_sign(sign); + self + } + /// Remove leading zeroes, consuming self and returning a new object. /// /// # Examples @@ -1087,7 +1150,7 @@ impl FixedDecimal { /// assert_eq!("2", dec.to_string()); /// ``` pub fn ceil(&mut self, position: i16) { - if self.is_negative { + if self.sign == Sign::Negative { self.truncate_right(position); return; } @@ -1144,7 +1207,7 @@ impl FixedDecimal { /// assert_eq!("2", dec.to_string()); /// ``` pub fn half_ceil(&mut self, position: i16) { - if self.is_negative { + if self.sign == Sign::Negative { self.half_truncate_right(position); return; } @@ -1201,7 +1264,7 @@ impl FixedDecimal { /// assert_eq!("1", dec.to_string()); /// ``` pub fn floor(&mut self, position: i16) { - if self.is_negative { + if self.sign == Sign::Negative { self.expand(position); return; } @@ -1258,7 +1321,7 @@ impl FixedDecimal { /// assert_eq!("1", dec.to_string()); /// ``` pub fn half_floor(&mut self, position: i16) { - if self.is_negative { + if self.sign == Sign::Negative { self.half_expand(position); return; } @@ -1534,7 +1597,8 @@ impl FixedDecimal { /// ``` pub fn signum(&self) -> Signum { let is_zero = self.digits.is_empty(); - match (self.is_negative, is_zero) { + let is_negative = self.sign == Sign::Negative; + match (is_negative, is_zero) { (false, false) => Signum::AboveZero, (false, true) => Signum::PositiveZero, (true, false) => Signum::BelowZero, @@ -1604,8 +1668,10 @@ impl writeable::Writeable for FixedDecimal { /// assert_eq!("42", result); /// ``` fn write_to(&self, sink: &mut W) -> fmt::Result { - if self.is_negative { - sink.write_char('-')?; + match self.sign { + Sign::Negative => sink.write_char('-')?, + Sign::Positive => sink.write_char('+')?, + Sign::None => (), } for m in self.magnitude_range().rev() { if m == -1 { @@ -1636,7 +1702,7 @@ impl writeable::Writeable for FixedDecimal { fn write_len(&self) -> writeable::LengthHint { writeable::LengthHint::exact(1) + ((self.upper_magnitude as i32 - self.lower_magnitude as i32) as usize) - + (if self.is_negative { 1 } else { 0 }) + + (if self.sign == Sign::None { 0 } else { 1 }) + (if self.lower_magnitude < 0 { 1 } else { 0 }) } } @@ -1653,19 +1719,25 @@ impl FromStr for FixedDecimal { fn from_str(input_str: &str) -> Result { // input_str: the input string // no_sign_str: the input string when the sign is removed from it - // Check if the input string is "" or "-" - if input_str.is_empty() || input_str == "-" { + if input_str.is_empty() { return Err(Error::Syntax); } let input_str = input_str.as_bytes(); - #[allow(clippy::indexing_slicing)] // TODO(#1668) Clippy exceptions need docs or fixing. - let is_negative = input_str[0] == b'-'; - #[allow(clippy::indexing_slicing)] // TODO(#1668) Clippy exceptions need docs or fixing. - let no_sign_str = if is_negative { - &input_str[1..] - } else { + #[allow(clippy::indexing_slicing)] // The string is not empty. + let sign = match input_str[0] { + b'-' => Sign::Negative, + b'+' => Sign::Positive, + _ => Sign::None, + }; + #[allow(clippy::indexing_slicing)] // The string is not empty. + let no_sign_str = if sign == Sign::None { input_str + } else { + &input_str[1..] }; + if no_sign_str.is_empty() { + return Err(Error::Syntax); + } // Compute length of each string once and store it, so if you use that multiple times, // you don't compute it multiple times // has_dot: shows if your input has dot in it @@ -1728,7 +1800,7 @@ impl FromStr for FixedDecimal { // defining the output dec here and set its sign let mut dec = Self { - is_negative, + sign, ..Default::default() }; @@ -2267,6 +2339,10 @@ fn test_from_str() { input_str: "-00123400", magnitudes: [7, 5, 2, 0], }, + TestCase { + input_str: "+00123400", + magnitudes: [7, 5, 2, 0], + }, TestCase { input_str: "0.0123400", magnitudes: [0, -2, -5, -7], @@ -2327,6 +2403,10 @@ fn test_from_str() { input_str: "-0", magnitudes: [0, 0, 0, 0], }, + TestCase { + input_str: "+0", + magnitudes: [0, 0, 0, 0], + }, TestCase { input_str: "000", magnitudes: [2, 0, 0, 0], @@ -2590,6 +2670,10 @@ fn test_syntax_error() { input_str: "-", expected_err: Some(Error::Syntax), }, + TestCase { + input_str: "+", + expected_err: Some(Error::Syntax), + }, TestCase { input_str: "-1", expected_err: None, diff --git a/utils/fixed_decimal/src/lib.rs b/utils/fixed_decimal/src/lib.rs index be49aec0d45..7f352158ec2 100644 --- a/utils/fixed_decimal/src/lib.rs +++ b/utils/fixed_decimal/src/lib.rs @@ -59,6 +59,7 @@ mod uint_iterator; pub use decimal::DoublePrecision; pub use decimal::FixedDecimal; +pub use decimal::Sign; use displaydoc::Display; pub use signum::Signum;