From 3a4fe534c56f51e670531456a8ca77643fcb4376 Mon Sep 17 00:00:00 2001 From: Cheng-Yang Chou Date: Thu, 29 Jan 2026 09:35:47 +0800 Subject: [PATCH 1/2] refactor: remove unnecessary allocations in error formatting The previous implementation allocated a temporary Vector and multiple Strings just to format the context map. This commit switches to a streaming approach using `write!`, which writes directly to the formatter without heap allocations. No functional changes. Signed-off-by: Cheng-Yang Chou --- datasketches/src/error.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/datasketches/src/error.rs b/datasketches/src/error.rs index 31559d3..959a702 100644 --- a/datasketches/src/error.rs +++ b/datasketches/src/error.rs @@ -161,15 +161,12 @@ impl fmt::Display for Error { if !self.context.is_empty() { write!(f, ", context: {{ ")?; - write!( - f, - "{}", - self.context - .iter() - .map(|(k, v)| format!("{k}: {v}")) - .collect::>() - .join(", ") - )?; + for (i, (k, v)) in self.context.iter().enumerate() { + if i > 0 { + write!(f, ", ")?; + } + write!(f, "{}: {}", k, v)?; + } write!(f, " }}")?; } From 7d63c1d7590697ccbb2ea2022631c372e35ad5ed Mon Sep 17 00:00:00 2001 From: Cheng-Yang Chou Date: Thu, 29 Jan 2026 10:02:57 +0800 Subject: [PATCH 2/2] feat: add regression tests for error formatting Introduces unit tests to verify the correctness of the `Display` implementation for the `Error` struct. The new tests cover: 1. Basic error messages without context. 2. Complex messages with multiple context items. Suggested-by: @tisonkun Signed-off-by: Cheng-Yang Chou --- datasketches/src/error.rs | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/datasketches/src/error.rs b/datasketches/src/error.rs index 959a702..c5a32cb 100644 --- a/datasketches/src/error.rs +++ b/datasketches/src/error.rs @@ -179,3 +179,38 @@ impl fmt::Display for Error { } impl std::error::Error for Error {} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_format_consistency() { + // Case 1: Test basic error message + let err = Error::new(ErrorKind::InvalidArgument, "something went wrong"); + assert_eq!( + format!("{}", err), + "InvalidArgument => something went wrong", + "Basic error formatting mismatch" + ); + } + + #[test] + fn test_format_with_multiple_contexts() { + // Case 2: Test with multiple contexts + // This validates the comma separation logic which is prone to regression + let err = Error::new(ErrorKind::InvalidData, "parsing failed") + .with_context("index", 42) + .with_context("file", "foo"); + + let output = format!("{}", err); + + // Expected full string + let expected = "InvalidData, context: { index: 42, file: foo } => parsing failed"; + + assert_eq!( + output, expected, + "Formatted output with context does not match expectation" + ); + } +}