From 3c8e266c5c65aea2d821a86063055d80250eb813 Mon Sep 17 00:00:00 2001 From: Viktor Jansson Date: Mon, 11 Dec 2023 19:44:24 +0100 Subject: [PATCH 1/6] Add try_append and try_update to return Result-types --- src/serde_error.rs | 8 ++++++++ src/vec.rs | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/src/serde_error.rs b/src/serde_error.rs index 5a25995..605c323 100644 --- a/src/serde_error.rs +++ b/src/serde_error.rs @@ -4,3 +4,11 @@ pub enum IdentifiedVecOfSerdeFailure { #[error("Duplicate element at offset {0}")] DuplicateElementsAtIndex(usize), } + +#[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)] +pub enum InsertionFailure { + #[error("Duplicate element with same id found")] + ElementWithSameIDFound, + #[error("Duplicate element with same value found")] + ElementWithSameValueFound, +} diff --git a/src/vec.rs b/src/vec.rs index b9d4ab8..967cac0 100644 --- a/src/vec.rs +++ b/src/vec.rs @@ -1,3 +1,4 @@ +use crate::InsertionFailure; use std::collections::HashMap; use std::fmt::{Debug, Display}; use std::hash::{Hash, Hasher}; @@ -568,6 +569,28 @@ where other.into_iter().for_each(|i| _ = self.append(i)) } + /// Try Append a new member to the end of the `identified_vec`, if the `identified_vec` already contains the element a InsertionError will be returned. + /// + /// - Parameter item: The element to add to the `identified_vec`. + /// - Returns: Either a Ok() with a pair `(inserted, index)`, where `inserted` is a Boolean value indicating whether + /// the operation added a new element, and `index` is the index of `item` in the resulting + /// `identified_vec`. If the given ID or Value pre-exists within the collection the function call returns a InsertionFailure Error specifying the reason. + /// - Complexity: The operation is expected to perform O(1) copy, hash, and compare operations on + /// the `ID` type, if it implements high-quality hashing. + #[inline] + pub fn try_append(&mut self, element: Element) -> Result<(bool, usize), InsertionFailure> { + let id = self.id(&element); + if self.index_of_id(&id).is_some() { + return Err(InsertionFailure::ElementWithSameIDFound); + } + + if self.contains(&element) { + return Err(InsertionFailure::ElementWithSameValueFound); + } + + Ok(self.insert(element, self.end_index())) + } + /// Adds the given element to the `identified_vec` unconditionally, either appending it to the `identified_vec``, or /// replacing an existing value if it's already present. /// @@ -606,6 +629,27 @@ where .expect("Replaced old value"); } + /// Try to add the given element to the `identified_vec` unconditionally, either appending it to the `identified_vec``, or + /// replacing an existing value if it's already present. + /// + /// - Parameter item: The value to append or replace. + /// - Returns: A Result with either the original element that was replaced by this operation, or `None` if the value was + /// appended to the end of the collection. If the given element already exists within the collection the function call returns a InsertionFailure. + /// - Complexity: The operation is expected to perform amortized O(1) copy, hash, and compare + /// operations on the `ID` type, if it implements high-quality hashing. + #[inline] + pub fn try_update_or_append( + &mut self, + element: Element, + ) -> Result, InsertionFailure> { + let id = self.id(&element); + if self.index_of_id(&id).is_some() && self.contains(&element) { + return Err(InsertionFailure::ElementWithSameValueFound); + } + + Ok(self._update_value(element, id)) + } + /// Insert a new member to this identified_vec at the specified index, if the identified_vec doesn't already contain /// it. /// From c287a6be28dab5f882824f6f14b9ae4bd7e95010 Mon Sep 17 00:00:00 2001 From: Viktor Jansson Date: Tue, 12 Dec 2023 14:52:12 +0100 Subject: [PATCH 2/6] Rework functions and add tests --- src/serde_error.rs | 8 ++-- src/vec.rs | 93 ++++++++++++++++++++++++++++++---------------- tests/tests.rs | 77 +++++++++++++++++++++++++++++++++++++- 3 files changed, 142 insertions(+), 36 deletions(-) diff --git a/src/serde_error.rs b/src/serde_error.rs index 605c323..65a939b 100644 --- a/src/serde_error.rs +++ b/src/serde_error.rs @@ -6,9 +6,11 @@ pub enum IdentifiedVecOfSerdeFailure { } #[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)] -pub enum InsertionFailure { - #[error("Duplicate element with same id found")] - ElementWithSameIDFound, +pub enum Error { + #[error("Element with that id not found in collection")] + ExpectedElementNotPresent, #[error("Duplicate element with same value found")] ElementWithSameValueFound, + #[error("Duplicate element with same ID found")] + ElementWithSameIDFound, } diff --git a/src/vec.rs b/src/vec.rs index 967cac0..7283b94 100644 --- a/src/vec.rs +++ b/src/vec.rs @@ -1,4 +1,4 @@ -use crate::InsertionFailure; +use crate::serde_error::Error; use std::collections::HashMap; use std::fmt::{Debug, Display}; use std::hash::{Hash, Hasher}; @@ -535,6 +535,59 @@ where } } +impl IdentifiedVec +where + ID: Eq + Hash + Clone + Debug, + Element: Eq + Hash + Clone + Debug, +{ + /// Try append a new unique member to the end of the `identified_vec`, if the `identified_vec` already contains the Value or ID a Error will be returned. + /// + /// - Parameter item: The element to add to the `identified_vec`. + /// - Returns: Either a Ok() with a pair `(inserted, index)`, where `inserted` is a Boolean value indicating whether + /// the operation added a new element, and `index` is the index of `item` in the resulting + /// `identified_vec`. If the given ID already exist `Error::ElementWithSameIDFound` willl be returned and if the value pre-exists within the collection the function call returns `Error::ElementWithSameValueFound`. + /// - Complexity: The operation is expected to perform O(1) copy, hash, and compare operations on + /// the `ID` type, if it implements high-quality hashing. + #[inline] + pub fn try_append_unique_element(&mut self, element: Element) -> Result<(bool, usize), Error> { + let id = self.id(&element); + + if let Some(value) = self.get(&id) { + if value == &element { + return Err(Error::ElementWithSameValueFound); + } else { + return Err(Error::ElementWithSameIDFound); + } + } + + Ok(self.append(element)) + } +} + +impl IdentifiedVec +where + ID: Eq + Hash + Clone + Debug, +{ + /// Try append a new member to the end of the `identified_vec`, if the `identified_vec` already contains the element a Error will be returned. + /// + /// - Parameter item: The element to add to the `identified_vec`. + /// - Returns: Either a Ok() with a pair `(inserted, index)`, where `inserted` is a Boolean value indicating whether + /// the operation added a new element, and `index` is the index of `item` in the resulting + /// `identified_vec`. If the given ID pre-exists within the collection the function call returns `Error::ElementWithSameIDFound`. + /// - Complexity: The operation is expected to perform O(1) copy, hash, and compare operations on + /// the `ID` type, if it implements high-quality hashing. + #[inline] + pub fn try_append(&mut self, element: Element) -> Result<(bool, usize), Error> { + let id = self.id(&element); + + if self.contains_id(&id) { + return Err(Error::ElementWithSameIDFound); + } + + Ok(self.append(element)) + } +} + /////////////////////// //// Public Insert /// /////////////////////// @@ -569,28 +622,6 @@ where other.into_iter().for_each(|i| _ = self.append(i)) } - /// Try Append a new member to the end of the `identified_vec`, if the `identified_vec` already contains the element a InsertionError will be returned. - /// - /// - Parameter item: The element to add to the `identified_vec`. - /// - Returns: Either a Ok() with a pair `(inserted, index)`, where `inserted` is a Boolean value indicating whether - /// the operation added a new element, and `index` is the index of `item` in the resulting - /// `identified_vec`. If the given ID or Value pre-exists within the collection the function call returns a InsertionFailure Error specifying the reason. - /// - Complexity: The operation is expected to perform O(1) copy, hash, and compare operations on - /// the `ID` type, if it implements high-quality hashing. - #[inline] - pub fn try_append(&mut self, element: Element) -> Result<(bool, usize), InsertionFailure> { - let id = self.id(&element); - if self.index_of_id(&id).is_some() { - return Err(InsertionFailure::ElementWithSameIDFound); - } - - if self.contains(&element) { - return Err(InsertionFailure::ElementWithSameValueFound); - } - - Ok(self.insert(element, self.end_index())) - } - /// Adds the given element to the `identified_vec` unconditionally, either appending it to the `identified_vec``, or /// replacing an existing value if it's already present. /// @@ -633,21 +664,19 @@ where /// replacing an existing value if it's already present. /// /// - Parameter item: The value to append or replace. - /// - Returns: A Result with either the original element that was replaced by this operation, or `None` if the value was - /// appended to the end of the collection. If the given element already exists within the collection the function call returns a InsertionFailure. + /// - Returns: A Result with either the original element that was replaced by this operation, or a Error, `Error::ExpectedElementNotPresent`, specifying that the expected element is not present within the collection. /// - Complexity: The operation is expected to perform amortized O(1) copy, hash, and compare /// operations on the `ID` type, if it implements high-quality hashing. #[inline] - pub fn try_update_or_append( - &mut self, - element: Element, - ) -> Result, InsertionFailure> { + pub fn try_update(&mut self, element: Element) -> Result { let id = self.id(&element); - if self.index_of_id(&id).is_some() && self.contains(&element) { - return Err(InsertionFailure::ElementWithSameValueFound); + if self.get(&id).is_none() { + return Err(Error::ExpectedElementNotPresent); } - Ok(self._update_value(element, id)) + Ok(self + ._update_value(element, id) + .expect("Failed to update value")) } /// Insert a new member to this identified_vec at the specified index, if the identified_vec doesn't already contain diff --git a/tests/tests.rs b/tests/tests.rs index b415fdd..45d10ab 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -3,7 +3,7 @@ use std::{cell::RefCell, collections::HashSet, fmt::Debug, ops::Deref}; use identified_vec::{ - ConflictResolutionChoice, Identifiable, IdentifiedVec, IdentifiedVecOf, + ConflictResolutionChoice, Error, Identifiable, IdentifiedVec, IdentifiedVecOf, IdentifiedVecOfSerdeFailure, }; @@ -314,6 +314,59 @@ fn append() { assert_eq!(identified_vec.items(), [1, 2, 3, 4]); } +#[test] +fn try_append_unique_element() { + let mut identified_vec = SUT::from_iter([1, 2, 3]); + let result = identified_vec.try_append_unique_element(4); + assert!(result.is_ok()); + assert_eq!(result.unwrap().1, 3); + assert_eq!(identified_vec.items(), [1, 2, 3, 4]); + + let mut identified_vec = SUT::from_iter([1, 2, 3]); + let result = identified_vec.try_append_unique_element(2); + assert!(result.is_err()); + assert_eq!(result, Err(Error::ElementWithSameValueFound)); + assert_eq!(identified_vec.items(), [1, 2, 3]); +} + +#[test] +fn try_append() { + let mut identified_vec = SUT::from_iter([1, 2, 3]); + let result = identified_vec.try_append(4); + assert!(result.is_ok()); + assert_eq!(result.unwrap().1, 3); + assert_eq!(identified_vec.items(), [1, 2, 3, 4]); + + let mut identified_vec: Users = IdentifiedVecOf::new(); + identified_vec.append(User::blob()); + identified_vec.append(User::blob_jr()); + identified_vec.append(User::blob_sr()); + let result = identified_vec.try_append(User::new(2, "Blob Jr Jr")); + assert!(result.is_err()); + assert_eq!(result, Err(Error::ElementWithSameIDFound)); + assert_eq!( + identified_vec.items(), + [User::blob(), User::blob_jr(), User::blob_sr()] + ); + + let mut identified_vec: Users = IdentifiedVecOf::new(); + identified_vec.append(User::blob()); + identified_vec.append(User::blob_jr()); + identified_vec.append(User::blob_sr()); + let result = identified_vec.try_append(User::new(4, "Blob Jr Jr")); + assert!(result.is_ok()); + assert_eq!(result, Ok((true, 3))); + assert_eq!( + identified_vec.items(), + [ + User::blob(), + User::blob_jr(), + User::blob_sr(), + User::new(4, "Blob Jr Jr") + ] + ); +} + #[test] fn append_other() { let mut identified_vec = SUT::from_iter([1, 2, 3]); @@ -372,6 +425,28 @@ fn update_or_append() { assert_eq!(identified_vec.update_or_append(2), Some(2)); } +#[test] +fn try_update() { + let mut identified_vec = SUT::from_iter([1, 2, 3]); + assert_eq!( + identified_vec.try_update(4), + Err(Error::ExpectedElementNotPresent) + ); + assert_eq!(identified_vec.items(), [1, 2, 3]); + + let mut identified_vec: Users = IdentifiedVecOf::new(); + identified_vec.append(User::blob()); + identified_vec.append(User::blob_jr()); + identified_vec.append(User::blob_sr()); + let result = identified_vec.try_update(User::new(2, "Blob Jr Sr")); + assert!(result.is_ok()); + assert_eq!(result, Ok(User::blob_jr())); + assert_eq!( + identified_vec.items(), + [User::blob(), User::new(2, "Blob Jr Sr"), User::blob_sr()] + ); +} + #[test] fn update_or_insert() { let mut identified_vec = SUT::from_iter([1, 2, 3]); From 9c0ff6bbe081a3808dd8b16f2005b13b7974a423 Mon Sep 17 00:00:00 2001 From: Viktor Jansson Date: Tue, 12 Dec 2023 15:08:43 +0100 Subject: [PATCH 3/6] Add associated values to Error enum --- src/serde_error.rs | 12 ++++++------ src/vec.rs | 8 ++++---- tests/tests.rs | 6 +++--- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/serde_error.rs b/src/serde_error.rs index 65a939b..d38967a 100644 --- a/src/serde_error.rs +++ b/src/serde_error.rs @@ -7,10 +7,10 @@ pub enum IdentifiedVecOfSerdeFailure { #[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)] pub enum Error { - #[error("Element with that id not found in collection")] - ExpectedElementNotPresent, - #[error("Duplicate element with same value found")] - ElementWithSameValueFound, - #[error("Duplicate element with same ID found")] - ElementWithSameIDFound, + #[error("Element with that id: `{0}` not found in collection")] + ExpectedElementNotPresent(String), + #[error("Duplicate element with same value: `{0}` found")] + ElementWithSameValueFound(String), + #[error("Duplicate element with same ID: `{0}` found")] + ElementWithSameIDFound(String), } diff --git a/src/vec.rs b/src/vec.rs index 7283b94..95e605d 100644 --- a/src/vec.rs +++ b/src/vec.rs @@ -554,9 +554,9 @@ where if let Some(value) = self.get(&id) { if value == &element { - return Err(Error::ElementWithSameValueFound); + return Err(Error::ElementWithSameValueFound(format!("{:#?}", value))); } else { - return Err(Error::ElementWithSameIDFound); + return Err(Error::ElementWithSameIDFound(format!("{:#?}", id))); } } @@ -581,7 +581,7 @@ where let id = self.id(&element); if self.contains_id(&id) { - return Err(Error::ElementWithSameIDFound); + return Err(Error::ElementWithSameIDFound(format!("{:#?}", id))); } Ok(self.append(element)) @@ -671,7 +671,7 @@ where pub fn try_update(&mut self, element: Element) -> Result { let id = self.id(&element); if self.get(&id).is_none() { - return Err(Error::ExpectedElementNotPresent); + return Err(Error::ExpectedElementNotPresent(format!("{:#?}", id))); } Ok(self diff --git a/tests/tests.rs b/tests/tests.rs index 45d10ab..1c9a4cf 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -325,7 +325,7 @@ fn try_append_unique_element() { let mut identified_vec = SUT::from_iter([1, 2, 3]); let result = identified_vec.try_append_unique_element(2); assert!(result.is_err()); - assert_eq!(result, Err(Error::ElementWithSameValueFound)); + assert_eq!(result, Err(Error::ElementWithSameValueFound(format!("2")))); assert_eq!(identified_vec.items(), [1, 2, 3]); } @@ -343,7 +343,7 @@ fn try_append() { identified_vec.append(User::blob_sr()); let result = identified_vec.try_append(User::new(2, "Blob Jr Jr")); assert!(result.is_err()); - assert_eq!(result, Err(Error::ElementWithSameIDFound)); + assert_eq!(result, Err(Error::ElementWithSameIDFound(format!("2")))); assert_eq!( identified_vec.items(), [User::blob(), User::blob_jr(), User::blob_sr()] @@ -430,7 +430,7 @@ fn try_update() { let mut identified_vec = SUT::from_iter([1, 2, 3]); assert_eq!( identified_vec.try_update(4), - Err(Error::ExpectedElementNotPresent) + Err(Error::ExpectedElementNotPresent(format!("4"))) ); assert_eq!(identified_vec.items(), [1, 2, 3]); From 1438318a61deedf969fecd48b54b1fb0def15d82 Mon Sep 17 00:00:00 2001 From: Viktor Jansson Date: Tue, 12 Dec 2023 20:00:26 +0100 Subject: [PATCH 4/6] Minor fixes to documentation and tests --- src/vec.rs | 5 ++--- tests/tests.rs | 33 ++++++++++++++++++--------------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/vec.rs b/src/vec.rs index 95e605d..0912109 100644 --- a/src/vec.rs +++ b/src/vec.rs @@ -577,7 +577,7 @@ where /// - Complexity: The operation is expected to perform O(1) copy, hash, and compare operations on /// the `ID` type, if it implements high-quality hashing. #[inline] - pub fn try_append(&mut self, element: Element) -> Result<(bool, usize), Error> { + pub fn try_append_new(&mut self, element: Element) -> Result<(bool, usize), Error> { let id = self.id(&element); if self.contains_id(&id) { @@ -660,8 +660,7 @@ where .expect("Replaced old value"); } - /// Try to add the given element to the `identified_vec` unconditionally, either appending it to the `identified_vec``, or - /// replacing an existing value if it's already present. + /// Try to update the given element to the `identified_vec` if a element with the same ID is already present. /// /// - Parameter item: The value to append or replace. /// - Returns: A Result with either the original element that was replaced by this operation, or a Error, `Error::ExpectedElementNotPresent`, specifying that the expected element is not present within the collection. diff --git a/tests/tests.rs b/tests/tests.rs index 1c9a4cf..095480a 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -330,9 +330,9 @@ fn try_append_unique_element() { } #[test] -fn try_append() { +fn try_append_new_unique_element() { let mut identified_vec = SUT::from_iter([1, 2, 3]); - let result = identified_vec.try_append(4); + let result = identified_vec.try_append_new(4); assert!(result.is_ok()); assert_eq!(result.unwrap().1, 3); assert_eq!(identified_vec.items(), [1, 2, 3, 4]); @@ -341,19 +341,7 @@ fn try_append() { identified_vec.append(User::blob()); identified_vec.append(User::blob_jr()); identified_vec.append(User::blob_sr()); - let result = identified_vec.try_append(User::new(2, "Blob Jr Jr")); - assert!(result.is_err()); - assert_eq!(result, Err(Error::ElementWithSameIDFound(format!("2")))); - assert_eq!( - identified_vec.items(), - [User::blob(), User::blob_jr(), User::blob_sr()] - ); - - let mut identified_vec: Users = IdentifiedVecOf::new(); - identified_vec.append(User::blob()); - identified_vec.append(User::blob_jr()); - identified_vec.append(User::blob_sr()); - let result = identified_vec.try_append(User::new(4, "Blob Jr Jr")); + let result = identified_vec.try_append_new(User::new(4, "Blob Jr Jr")); assert!(result.is_ok()); assert_eq!(result, Ok((true, 3))); assert_eq!( @@ -367,6 +355,21 @@ fn try_append() { ); } +#[test] +fn try_append_element_with_existing_id() { + let mut identified_vec: Users = IdentifiedVecOf::new(); + identified_vec.append(User::blob()); + identified_vec.append(User::blob_jr()); + identified_vec.append(User::blob_sr()); + let result = identified_vec.try_append_new(User::new(2, "Blob Jr Jr")); + assert!(result.is_err()); + assert_eq!(result, Err(Error::ElementWithSameIDFound(format!("2")))); + assert_eq!( + identified_vec.items(), + [User::blob(), User::blob_jr(), User::blob_sr()] + ); +} + #[test] fn append_other() { let mut identified_vec = SUT::from_iter([1, 2, 3]); From 528dc35998732938a084b0b34b31113b698a077e Mon Sep 17 00:00:00 2001 From: Viktor Jansson Date: Wed, 13 Dec 2023 20:22:58 +0100 Subject: [PATCH 5/6] Reduce number of contraints for try_append_unique_element --- src/vec.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vec.rs b/src/vec.rs index 0912109..506d818 100644 --- a/src/vec.rs +++ b/src/vec.rs @@ -538,7 +538,7 @@ where impl IdentifiedVec where ID: Eq + Hash + Clone + Debug, - Element: Eq + Hash + Clone + Debug, + Element: Eq + Debug, { /// Try append a new unique member to the end of the `identified_vec`, if the `identified_vec` already contains the Value or ID a Error will be returned. /// @@ -554,9 +554,9 @@ where if let Some(value) = self.get(&id) { if value == &element { - return Err(Error::ElementWithSameValueFound(format!("{:#?}", value))); + return Err(Error::ElementWithSameValueFound(format!("{:?}", value))); } else { - return Err(Error::ElementWithSameIDFound(format!("{:#?}", id))); + return Err(Error::ElementWithSameIDFound(format!("{:?}", id))); } } From 63b4126a2724be98a812212e7a5bbb8d30892438 Mon Sep 17 00:00:00 2001 From: Viktor Jansson Date: Wed, 13 Dec 2023 22:35:07 +0100 Subject: [PATCH 6/6] Add 100% test coverage --- tests/tests.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/tests.rs b/tests/tests.rs index 095480a..8354457 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -327,6 +327,16 @@ fn try_append_unique_element() { assert!(result.is_err()); assert_eq!(result, Err(Error::ElementWithSameValueFound(format!("2")))); assert_eq!(identified_vec.items(), [1, 2, 3]); + + let mut identified_vec = + IdentifiedVecOf::from_iter([User::blob(), User::blob_jr(), User::blob_sr()]); + let result = identified_vec.try_append_unique_element(User::new(2, "Blob blob Jr")); + assert!(result.is_err()); + assert_eq!(result, Err(Error::ElementWithSameIDFound(format!("2")))); + assert_eq!( + identified_vec.items(), + [User::blob(), User::blob_jr(), User::blob_sr()] + ); } #[test]