diff --git a/.jules/bolt.md b/.jules/bolt.md index 9d407c0..1878ef8 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -5,3 +5,7 @@ ## 2024-05-25 - Avoid double lookup and lock contention in Context lookup **Learning:** `HashMap::get(name)` followed by `.unwrap()` inside a Mutex lock not only does double lookup but keeps the lock longer than necessary when executing an inner function or cloning a value. **Action:** Use a single lookup, clone the value or arc into a local variable (`Option`), and release the lock immediately before execution. + +## 2024-05-26 - Optimize decimal conversion in Value +**Learning:** `rust_decimal::Decimal` allows efficient and direct conversion to basic types like `i64` and `f64` via `to_i64` and `to_f64` using the `rust_decimal::prelude::ToPrimitive` trait. Converting to string and then parsing `val.to_string().parse()` is an anti-pattern as it incurs heap allocation overhead, which is bad for performance. When doing integer conversions from `Decimal`, it's critical to check `val.scale() == 0` first to maintain behavioral parity with string parsing (which would reject floats). +**Action:** Always favor direct conversion traits like `rust_decimal::prelude::ToPrimitive` methods over stringification when converting `Decimal` values to native numeric types. Keep edge cases like `scale()` properties in mind when changing conversion methods. diff --git a/src/value.rs b/src/value.rs index b4c3384..d1b9133 100644 --- a/src/value.rs +++ b/src/value.rs @@ -96,20 +96,20 @@ impl Value { pub fn integer(self) -> Result { match self { - Self::Number(val) => val - .to_string() - .parse() - .map_or(Err(Error::InvalidInteger), |num| Ok(num)), + Self::Number(val) => { + if val.scale() == 0 { + val.to_i64().ok_or(Error::InvalidInteger) + } else { + Err(Error::InvalidInteger) + } + } _ => Err(Error::InvalidInteger), } } pub fn float(self) -> Result { match self { - Self::Number(val) => val - .to_string() - .parse() - .map_or(Err(Error::InvalidFloat), |num| Ok(num)), + Self::Number(val) => val.to_f64().ok_or(Error::InvalidFloat), _ => Err(Error::InvalidFloat), } } @@ -148,3 +148,35 @@ impl_value_from_for_number!( [f64, from_f64], [f32, from_f32] ); + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_value_integer() { + assert_eq!(Value::from(10).integer().unwrap(), 10); + assert_eq!(Value::from(-5).integer().unwrap(), -5); + + // Test that integer() fails when scale is not 0 + let dec = Decimal::from_str("10.5").unwrap(); + assert!(Value::Number(dec).integer().is_err()); + + // Even if the value represents an integer, but has a non-zero scale, it should fail to parse + // according to strict behavior checking `val.scale() == 0` + let dec_with_scale = Decimal::from_str("10.0").unwrap(); + assert!(Value::Number(dec_with_scale).integer().is_err()); + } + + #[test] + fn test_value_float() { + assert_eq!(Value::from(10).float().unwrap(), 10.0); + assert_eq!(Value::from(-5).float().unwrap(), -5.0); + + let dec = Decimal::from_str("10.5").unwrap(); + assert_eq!(Value::Number(dec).float().unwrap(), 10.5); + + let dec_with_scale = Decimal::from_str("10.0").unwrap(); + assert_eq!(Value::Number(dec_with_scale).float().unwrap(), 10.0); + } +}