From 6af5267b1b00f8c4ec2ff8b544bad15fe3079757 Mon Sep 17 00:00:00 2001 From: Alexander Cyon Date: Sun, 10 Dec 2023 10:14:03 +0100 Subject: [PATCH 1/4] documentation improvements --- src/identifiable_trait.rs | 11 +++++++++++ src/lib.rs | 2 ++ src/vec_of.rs | 11 +++++++++++ 3 files changed, 24 insertions(+) diff --git a/src/identifiable_trait.rs b/src/identifiable_trait.rs index 76ac982..db68826 100644 --- a/src/identifiable_trait.rs +++ b/src/identifiable_trait.rs @@ -1,7 +1,18 @@ use std::fmt::Debug; use std::hash::Hash; +/// The `Identifiable` trait allows you to use the +/// `IdentifiedVecOf instead of the more verbose +/// `IdentifiedVec` but also allows you to +/// skip the `id_of_element: fn(&Element) -> ID` closure when +/// initializing a new identified vec. pub trait Identifiable { + /// The type that your `Element` will use as its globally unique and stable ID, + /// must impl `Hash` since it is used as a key in `IdentifiedVecOf`'s internal + /// `HashMap`. Must impl `Clone` since we need to be able to clone it as a key type ID: Eq + Hash + Clone + Debug; + + /// Return `Element`'s globally unique and stable ID, used to uniquely identify + /// the `Element` in the `IdentifiedVecOf` collection of elements. fn id(&self) -> Self::ID; } diff --git a/src/lib.rs b/src/lib.rs index 95cfdd8..5c7c2a2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -124,6 +124,8 @@ //! ``` //! extern crate identified_vec; //! use identified_vec::{IdentifiedVec, IdentifiedVecOf, Identifiable}; +//! +//! // closure which plucks out an ID from an element. //! let numbers = IdentifiedVec::::new_identifying_element(|e| *e); //! ``` diff --git a/src/vec_of.rs b/src/vec_of.rs index 970b391..67992a3 100644 --- a/src/vec_of.rs +++ b/src/vec_of.rs @@ -1,5 +1,7 @@ use anyerror::AnyError; use std::collections::HashMap; + +#[cfg(feature = "serde")] use std::fmt::Debug; #[cfg(feature = "serde")] @@ -15,6 +17,15 @@ use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; /////////////////////// /// IdentifiedVecOf /// /////////////////////// + +/// A type alias for `IdentifiedVec`, this is the +/// preferred and most powerful collection type of this crate, requires +/// that your `Element`s impl the `Identifiable` trait. Using this collection +/// allows you to skip passing the `id_of_element: fn(&Element) -> ID` closure +/// which you otherwise need to pass when initializing an `IdentifiedVec`. Using +/// `IdentifiedVecOf` together with feature "serde" also gives serde +/// serialization/deserialization as if it were a `Vec`, given that +/// `Element` implements serde serialization/deserialization of course. pub type IdentifiedVecOf = IdentifiedVec<::ID, Element>; ////////////////////////////////////////////// From f2b7ca7cfa07a000f0d31b597b20a6296be5d6d5 Mon Sep 17 00:00:00 2001 From: Alexander Cyon Date: Sun, 10 Dec 2023 11:39:52 +0100 Subject: [PATCH 2/4] Remove dependency anyerror, use generic in constructor methods instead --- Cargo.lock | 10 ---------- Cargo.toml | 1 - src/serde_error.rs | 2 +- src/vec.rs | 10 ++++------ src/vec_of.rs | 19 ++++++++----------- tests/tests.rs | 39 ++++++++++++++++----------------------- 6 files changed, 29 insertions(+), 52 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 284b98d..75506d3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,15 +2,6 @@ # It is not intended for manual editing. version = 3 -[[package]] -name = "anyerror" -version = "0.1.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bcd04a72664a65fb9adeae7ced0c98efd68a2b7a45adda8319b3bb36538404b8" -dependencies = [ - "serde", -] - [[package]] name = "cfg-if" version = "1.0.0" @@ -32,7 +23,6 @@ dependencies = [ name = "identified_vec" version = "0.1.3" dependencies = [ - "anyerror", "identified_vec", "maplit", "rand", diff --git a/Cargo.toml b/Cargo.toml index cb6aaed..fbeaafa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,6 @@ serde = ["dep:serde"] id_prim = [] [dependencies] -anyerror = "0.1.12" serde = { version = "1.0.193", optional = true } thiserror = "1.0.50" diff --git a/src/serde_error.rs b/src/serde_error.rs index 408f048..5a25995 100644 --- a/src/serde_error.rs +++ b/src/serde_error.rs @@ -1,5 +1,5 @@ #[cfg(feature = "serde")] -#[derive(thiserror::Error, Debug, Clone)] +#[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)] pub enum IdentifiedVecOfSerdeFailure { #[error("Duplicate element at offset {0}")] DuplicateElementsAtIndex(usize), diff --git a/src/vec.rs b/src/vec.rs index 5f27f04..37f3910 100644 --- a/src/vec.rs +++ b/src/vec.rs @@ -2,8 +2,6 @@ use std::collections::HashMap; use std::fmt::{Debug, Display}; use std::hash::{Hash, Hasher}; -use anyerror::AnyError; - /// Representation of a choice in a conflict resolution /// where two elements with the same ID exists, if `ChooseFirst`, /// is specified the first/current/existing value will be used, but @@ -236,11 +234,11 @@ where /// implements high-quality hashing. #[cfg(not(tarpaulin_include))] // false negative #[inline] - pub fn new_from_iter_try_uniquing_ids_with( + pub fn try_from_iter_select_unique_ids_with( elements: I, id_of_element: fn(&Element) -> ID, - combine: fn((usize, &Element, &Element)) -> Result, - ) -> Result + combine: fn((usize, &Element, &Element)) -> Result, + ) -> Result where I: IntoIterator, { @@ -294,7 +292,7 @@ where /// implements high-quality hashing. #[cfg(not(tarpaulin_include))] // false negative #[inline] - pub fn new_from_iter_uniquing_ids_with( + pub fn from_iter_select_unique_ids_with( elements: I, id_of_element: fn(&Element) -> ID, combine: fn((usize, &Element, &Element)) -> ConflictResolutionChoice, diff --git a/src/vec_of.rs b/src/vec_of.rs index 67992a3..3aa82bb 100644 --- a/src/vec_of.rs +++ b/src/vec_of.rs @@ -1,4 +1,3 @@ -use anyerror::AnyError; use std::collections::HashMap; #[cfg(feature = "serde")] @@ -84,14 +83,14 @@ where /// - Complexity: Expected O(*n*) on average, where *n* is the count of elements, if `ID` /// implements high-quality hashing. #[inline] - pub fn new_from_iter_try_uniquing_with( + pub fn tryfrom_iter_select_unique_with( elements: I, - combine: fn((usize, &Element, &Element)) -> Result, - ) -> Result + combine: fn((usize, &Element, &Element)) -> Result, + ) -> Result where I: IntoIterator, { - Self::new_from_iter_try_uniquing_ids_with(elements, |e| e.id(), combine) + Self::try_from_iter_select_unique_ids_with(elements, |e| e.id(), combine) } /// Creates a new `identified_vec` from the elements in the given sequence, using a combining closure to @@ -110,14 +109,14 @@ where /// - Complexity: Expected O(*n*) on average, where *n* is the count of elements, if `ID` /// implements high-quality hashing. #[inline] - pub fn new_from_iter_uniquing_with( + pub fn from_iter_select_unique_with( elements: I, combine: fn((usize, &Element, &Element)) -> ConflictResolutionChoice, ) -> Self where I: IntoIterator, { - Self::new_from_iter_uniquing_ids_with(elements, |e| e.id(), combine) + Self::from_iter_select_unique_ids_with(elements, |e| e.id(), combine) } } @@ -147,10 +146,8 @@ where deserializer: D, ) -> Result, D::Error> { let elements = Vec::::deserialize(deserializer)?; - IdentifiedVecOf::::new_from_iter_try_uniquing_with(elements, |(idx, _, _)| { - Err(AnyError::new( - &IdentifiedVecOfSerdeFailure::DuplicateElementsAtIndex(idx), - )) + IdentifiedVecOf::::tryfrom_iter_select_unique_with(elements, |(idx, _, _)| { + Err(IdentifiedVecOfSerdeFailure::DuplicateElementsAtIndex(idx)) }) .map_err(de::Error::custom) } diff --git a/tests/tests.rs b/tests/tests.rs index 6cb5ee4..081259e 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -2,7 +2,6 @@ use std::{cell::RefCell, collections::HashSet, fmt::Debug, ops::Deref}; -use anyerror::AnyError; use identified_vec::{ ConflictResolutionChoice, Identifiable, IdentifiedVec, IdentifiedVecOf, IdentifiedVecOfSerdeFailure, @@ -217,7 +216,7 @@ fn constructor_id_uniquing_elements() { } } - let conservative = IdentifiedVec::::new_from_iter_uniquing_ids_with( + let conservative = IdentifiedVec::::from_iter_select_unique_ids_with( [ Model::new(1, "A"), Model::new(2, "B"), @@ -232,7 +231,7 @@ fn constructor_id_uniquing_elements() { [&Model::new(1, "A"), &Model::new(2, "B")] ); - let progressive = IdentifiedVec::::new_from_iter_uniquing_ids_with( + let progressive = IdentifiedVec::::from_iter_select_unique_ids_with( [ Model::new(1, "A"), Model::new(2, "B"), @@ -249,7 +248,7 @@ fn constructor_id_uniquing_elements() { } #[test] -fn constructor_uniquing_elements() { +fn constructor_from_iter_select_unique_with() { #[derive(Eq, PartialEq, Clone, Hash, Debug)] struct Model { id: i32, @@ -268,7 +267,7 @@ fn constructor_uniquing_elements() { } } - let conservative = IdentifiedVecOf::::new_from_iter_uniquing_with( + let conservative = IdentifiedVecOf::::from_iter_select_unique_with( [ Model::new(1, "A"), Model::new(2, "B"), @@ -282,7 +281,7 @@ fn constructor_uniquing_elements() { [&Model::new(1, "A"), &Model::new(2, "B")] ); - let progressive = IdentifiedVecOf::::new_from_iter_uniquing_with( + let progressive = IdentifiedVecOf::::from_iter_select_unique_with( [ Model::new(1, "A"), Model::new(2, "B"), @@ -413,7 +412,7 @@ fn serde() { serde_json::from_str::("[1,1,1]") .expect_err("should fail") .to_string(), - "identified_vec::serde_error::IdentifiedVecOfSerdeFailure: Duplicate element at offset 1" + "Duplicate element at offset 1" ); assert!(serde_json::from_str::("invalid").is_err(),); @@ -457,40 +456,34 @@ fn eq() { let mut vecs: Vec> = vec![ IdentifiedVecOf::new(), IdentifiedVecOf::new_identifying_element(|e| e.id()), - IdentifiedVecOf::new_from_iter_uniquing_with([], |_| ConflictResolutionChoice::ChooseLast), - IdentifiedVecOf::new_from_iter_uniquing_ids_with( + IdentifiedVecOf::from_iter_select_unique_with([], |_| ConflictResolutionChoice::ChooseLast), + IdentifiedVecOf::from_iter_select_unique_ids_with( [], |e| e.id(), |_| ConflictResolutionChoice::ChooseLast, ), - IdentifiedVecOf::new_from_iter_try_uniquing_ids_with( + IdentifiedVecOf::try_from_iter_select_unique_ids_with( [], |e: &Foo| e.id(), - |_| Ok(ConflictResolutionChoice::ChooseLast), + |_| Result::<_, ()>::Ok(ConflictResolutionChoice::ChooseLast), ) .unwrap(), ]; assert_eq!( - IdentifiedVecOf::new_from_iter_try_uniquing_ids_with( + IdentifiedVecOf::try_from_iter_select_unique_ids_with( [Foo::new(), Foo::new()], |e: &Foo| e.id(), - |_| Err(AnyError::new( - &IdentifiedVecOfSerdeFailure::DuplicateElementsAtIndex(1) - )), + |_| Err(IdentifiedVecOfSerdeFailure::DuplicateElementsAtIndex(1)), ), - Err(AnyError::new( - &IdentifiedVecOfSerdeFailure::DuplicateElementsAtIndex(1) - )) + Err(IdentifiedVecOfSerdeFailure::DuplicateElementsAtIndex(1)) ); assert_eq!( - IdentifiedVecOf::new_from_iter_try_uniquing_with([Foo::new(), Foo::new()], |_| Err( - AnyError::new(&IdentifiedVecOfSerdeFailure::DuplicateElementsAtIndex(1)) + IdentifiedVecOf::tryfrom_iter_select_unique_with([Foo::new(), Foo::new()], |_| Err( + IdentifiedVecOfSerdeFailure::DuplicateElementsAtIndex(1) ),), - Err(AnyError::new( - &IdentifiedVecOfSerdeFailure::DuplicateElementsAtIndex(1) - )) + Err(IdentifiedVecOfSerdeFailure::DuplicateElementsAtIndex(1)) ); vecs.iter().for_each(|l| { From 5920d6f52843a3adb0da12e12ff05ea1055e578b Mon Sep 17 00:00:00 2001 From: Alexander Cyon Date: Sun, 10 Dec 2023 11:41:28 +0100 Subject: [PATCH 3/4] bump version number --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 75506d3..12f4005 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -21,7 +21,7 @@ dependencies = [ [[package]] name = "identified_vec" -version = "0.1.3" +version = "0.1.4" dependencies = [ "identified_vec", "maplit", diff --git a/Cargo.toml b/Cargo.toml index fbeaafa..ed3ce7a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "identified_vec" -version = "0.1.3" +version = "0.1.4" edition = "2021" authors = ["Alexander Cyon "] description = "Like HashSet but retaining INSERTION order and without `Hash` requirement on the Element type." From 8c88b38202227554cb1a36750581f2dea70b408d Mon Sep 17 00:00:00 2001 From: Alexander Cyon Date: Sun, 10 Dec 2023 11:51:26 +0100 Subject: [PATCH 4/4] add fn items(&self) -> Vec --- src/vec.rs | 30 ++++++++++++++++++++++++------ src/vec_of.rs | 4 ++-- tests/tests.rs | 47 ++++++++++++++++++++++++++--------------------- 3 files changed, 52 insertions(+), 29 deletions(-) diff --git a/src/vec.rs b/src/vec.rs index 37f3910..b9d4ab8 100644 --- a/src/vec.rs +++ b/src/vec.rs @@ -404,16 +404,16 @@ where self.elements.get_mut(id) } - /// A read-only collection view for the elements contained in this array, as a `Vec`. + /// A read-only collection of references to the elements contained in this array, as a `Vec<&Elements>`. + /// + /// N.B. that this method is not constant time. + /// + /// If `Element` implements `Clone` you can use `self.items()` which returns a `Vec`, of cloned elements. /// /// - Complexity: O(n) #[inline] pub fn elements(&self) -> Vec<&Element> { - let mut elements_ordered = Vec::<&Element>::new(); - for id in &self.order { - elements_ordered.push(self.elements.get(id).expect("element")); - } - elements_ordered + self.iter().collect() } /// Returns `true` if the `identified_vec` contains the `element.` @@ -441,6 +441,24 @@ where } } +impl IdentifiedVec +where + Element: Clone, + ID: Eq + Hash + Clone + Debug, +{ + /// A read-only collection of clones of the elements contained in this array, as a `Vec`. + /// + /// N.B. that this method is not constant time. + /// + /// Use `self.elements()` if you are looking for a collection of references. + /// + /// - Complexity: O(n) + #[inline] + pub fn items(&self) -> Vec { + self.iter().map(|e| e.clone()).collect() + } +} + /// An iterator over the items of an `IdentifiedVec`. pub struct IdentifiedVecIterator<'a, ID, Element> where diff --git a/src/vec_of.rs b/src/vec_of.rs index 3aa82bb..f6801c1 100644 --- a/src/vec_of.rs +++ b/src/vec_of.rs @@ -83,7 +83,7 @@ where /// - Complexity: Expected O(*n*) on average, where *n* is the count of elements, if `ID` /// implements high-quality hashing. #[inline] - pub fn tryfrom_iter_select_unique_with( + pub fn try_from_iter_select_unique_with( elements: I, combine: fn((usize, &Element, &Element)) -> Result, ) -> Result @@ -146,7 +146,7 @@ where deserializer: D, ) -> Result, D::Error> { let elements = Vec::::deserialize(deserializer)?; - IdentifiedVecOf::::tryfrom_iter_select_unique_with(elements, |(idx, _, _)| { + IdentifiedVecOf::::try_from_iter_select_unique_with(elements, |(idx, _, _)| { Err(IdentifiedVecOfSerdeFailure::DuplicateElementsAtIndex(idx)) }) .map_err(de::Error::custom) diff --git a/tests/tests.rs b/tests/tests.rs index 081259e..b415fdd 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -193,18 +193,18 @@ fn index_id() { fn remove_element() { let mut identified_vec = SUT::from_iter([1, 2, 3]); assert_eq!(identified_vec.remove(&2), Some(2)); - assert_eq!(identified_vec.elements(), [&1, &3]); + assert_eq!(identified_vec.items(), [1, 3]); } #[test] fn remove_by_id() { let mut identified_vec = SUT::from_iter([1, 2, 3]); assert_eq!(identified_vec.remove_by_id(&2), Some(2)); - assert_eq!(identified_vec.elements(), [&1, &3]); + assert_eq!(identified_vec.items(), [1, 3]); } #[test] -fn constructor_id_uniquing_elements() { +fn constructor_from_iter_select_unique_ids_with() { #[derive(Eq, PartialEq, Clone, Hash, Debug)] struct Model { id: i32, @@ -227,8 +227,8 @@ fn constructor_id_uniquing_elements() { ); assert_eq!( - conservative.elements(), - [&Model::new(1, "A"), &Model::new(2, "B")] + conservative.items(), + [Model::new(1, "A"), Model::new(2, "B")] ); let progressive = IdentifiedVec::::from_iter_select_unique_ids_with( @@ -242,8 +242,8 @@ fn constructor_id_uniquing_elements() { ); assert_eq!( - progressive.elements(), - [&Model::new(1, "AAAA"), &Model::new(2, "B")] + progressive.items(), + [Model::new(1, "AAAA"), Model::new(2, "B")] ) } @@ -277,8 +277,13 @@ fn constructor_from_iter_select_unique_with() { ); assert_eq!( - conservative.elements(), - [&Model::new(1, "A"), &Model::new(2, "B")] + conservative.items(), + [Model::new(1, "A"), Model::new(2, "B")] + ); + + assert_eq!( + conservative.items(), + [Model::new(1, "A"), Model::new(2, "B")] ); let progressive = IdentifiedVecOf::::from_iter_select_unique_with( @@ -291,8 +296,8 @@ fn constructor_from_iter_select_unique_with() { ); assert_eq!( - progressive.elements(), - [&Model::new(1, "AAAA"), &Model::new(2, "B")] + progressive.items(), + [Model::new(1, "AAAA"), Model::new(2, "B")] ) } @@ -302,18 +307,18 @@ fn append() { let (mut inserted, mut index) = identified_vec.append(4); assert!(inserted); assert_eq!(index, 3); - assert_eq!(identified_vec.elements(), [&1, &2, &3, &4]); + assert_eq!(identified_vec.items(), [1, 2, 3, 4]); (inserted, index) = identified_vec.append(2); assert_eq!(inserted, false); assert_eq!(index, 1); - assert_eq!(identified_vec.elements(), [&1, &2, &3, &4]); + assert_eq!(identified_vec.items(), [1, 2, 3, 4]); } #[test] fn append_other() { let mut identified_vec = SUT::from_iter([1, 2, 3]); identified_vec.append_other([1, 4, 3, 5]); - assert_eq!(identified_vec.elements(), [&1, &2, &3, &4, &5]) + assert_eq!(identified_vec.items(), [1, 2, 3, 4, 5]) } #[test] @@ -322,11 +327,11 @@ fn insert() { let (mut inserted, mut index) = identified_vec.insert(0, 0); assert!(inserted); assert_eq!(index, 0); - assert_eq!(identified_vec.elements(), [&0, &1, &2, &3]); + assert_eq!(identified_vec.items(), [0, 1, 2, 3]); (inserted, index) = identified_vec.insert(2, 0); assert_eq!(inserted, false); assert_eq!(index, 2); - assert_eq!(identified_vec.elements(), [&0, &1, &2, &3]); + assert_eq!(identified_vec.items(), [0, 1, 2, 3]); } #[test] @@ -363,7 +368,7 @@ fn update_at_expect_panic_other_id() { fn update_or_append() { let mut identified_vec = SUT::from_iter([1, 2, 3]); assert_eq!(identified_vec.update_or_append(4), None); - assert_eq!(identified_vec.elements(), [&1, &2, &3, &4]); + assert_eq!(identified_vec.items(), [1, 2, 3, 4]); assert_eq!(identified_vec.update_or_append(2), Some(2)); } @@ -373,18 +378,18 @@ fn update_or_insert() { let (mut original_member, mut index) = identified_vec.update_or_insert(0, 0); assert_eq!(original_member, None); assert_eq!(index, 0); - assert_eq!(identified_vec.elements(), [&0, &1, &2, &3]); + assert_eq!(identified_vec.items(), [0, 1, 2, 3]); (original_member, index) = identified_vec.update_or_insert(2, 0); assert_eq!(original_member, Some(2)); assert_eq!(index, 2); - assert_eq!(identified_vec.elements(), [&0, &1, &2, &3]) + assert_eq!(identified_vec.items(), [0, 1, 2, 3]) } #[test] fn remove_at_offsets() { let mut identified_vec = SUT::from_iter([1, 2, 3]); identified_vec.remove_at_offsets([0, 2]); - assert_eq!(identified_vec.elements(), [&2]) + assert_eq!(identified_vec.items(), [2]) } #[test] @@ -480,7 +485,7 @@ fn eq() { ); assert_eq!( - IdentifiedVecOf::tryfrom_iter_select_unique_with([Foo::new(), Foo::new()], |_| Err( + IdentifiedVecOf::try_from_iter_select_unique_with([Foo::new(), Foo::new()], |_| Err( IdentifiedVecOfSerdeFailure::DuplicateElementsAtIndex(1) ),), Err(IdentifiedVecOfSerdeFailure::DuplicateElementsAtIndex(1))