From a77fe739934ea7e4bb41a4994c02c25d7aa8d94e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 8 Nov 2023 11:52:28 +0100 Subject: [PATCH 1/2] Fix variable name --- crates/fj-core/src/objects/handles.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/objects/handles.rs b/crates/fj-core/src/objects/handles.rs index 6f454c1de..7fc40275c 100644 --- a/crates/fj-core/src/objects/handles.rs +++ b/crates/fj-core/src/objects/handles.rs @@ -168,7 +168,7 @@ impl Handles { pub fn replace_with_multiple( &self, original: &Handle, - replacement: [Handle; N], + replacements: [Handle; N], ) -> Option where T: Debug + Ord, @@ -198,7 +198,13 @@ impl Handles { // Let's make that a bit more explicit by renaming the variable. let after = iter; - Some(before.into_iter().chain(replacement).chain(after).collect()) + Some( + before + .into_iter() + .chain(replacements) + .chain(after) + .collect(), + ) } } From bf66e1e33a584188c0b383dd8be7e21f5c4c5cc4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 8 Nov 2023 11:54:43 +0100 Subject: [PATCH 2/2] Merge `replace_with_multiple` into `replace` Having two different versions of the same method is not justified, in my opinion: - The convenience of having a single-replacement special version is so minimal, it doesn't justify any additional complexity. - The minimal win in convenience is more than made up for by the inconvenience of a longer name for the general version. Having a single, general `replace` method is much better. --- crates/fj-core/src/objects/handles.rs | 24 +------------------ crates/fj-core/src/operations/update/cycle.rs | 4 ++-- .../fj-core/src/operations/update/region.rs | 4 ++-- crates/fj-core/src/operations/update/shell.rs | 4 ++-- .../fj-core/src/operations/update/sketch.rs | 4 ++-- crates/fj-core/src/operations/update/solid.rs | 4 ++-- 6 files changed, 11 insertions(+), 33 deletions(-) diff --git a/crates/fj-core/src/objects/handles.rs b/crates/fj-core/src/objects/handles.rs index 7fc40275c..c3126ebb5 100644 --- a/crates/fj-core/src/objects/handles.rs +++ b/crates/fj-core/src/objects/handles.rs @@ -143,29 +143,7 @@ impl Handles { /// /// Panics, if the update results in a duplicate item. #[must_use] - pub fn replace( - &self, - original: &Handle, - replacement: Handle, - ) -> Option - where - T: Debug + Ord, - { - self.replace_with_multiple(original, [replacement]) - } - - /// Create a new instance in which the provided item has been replaced - /// - /// Returns `None`, if the provided item is not present. - /// - /// This is a more general version of [`Handles::replace`] which can replace - /// a single item with multiple others. - /// - /// # Panics - /// - /// Panics, if the replacement results in a duplicate item. - #[must_use] - pub fn replace_with_multiple( + pub fn replace( &self, original: &Handle, replacements: [Handle; N], diff --git a/crates/fj-core/src/operations/update/cycle.rs b/crates/fj-core/src/operations/update/cycle.rs index 2ad9e97de..f0dd04ce5 100644 --- a/crates/fj-core/src/operations/update/cycle.rs +++ b/crates/fj-core/src/operations/update/cycle.rs @@ -60,7 +60,7 @@ impl UpdateCycle for Cycle { ) -> Self { let edges = self .half_edges() - .replace(handle, update(handle)) + .replace(handle, [update(handle)]) .expect("Half-edge not found"); Cycle::new(edges) } @@ -72,7 +72,7 @@ impl UpdateCycle for Cycle { ) -> Self { let edges = self .half_edges() - .replace_with_multiple(handle, replace(handle)) + .replace(handle, replace(handle)) .expect("Half-edge not found"); Cycle::new(edges) } diff --git a/crates/fj-core/src/operations/update/region.rs b/crates/fj-core/src/operations/update/region.rs index d9613611c..8403d8ab9 100644 --- a/crates/fj-core/src/operations/update/region.rs +++ b/crates/fj-core/src/operations/update/region.rs @@ -75,7 +75,7 @@ impl UpdateRegion for Region { ) -> Self { let interiors = self .interiors() - .replace(handle, update(handle)) + .replace(handle, [update(handle)]) .expect("Cycle not found"); Region::new(self.exterior().clone(), interiors, self.color()) } @@ -87,7 +87,7 @@ impl UpdateRegion for Region { ) -> Self { let interiors = self .interiors() - .replace_with_multiple(handle, replace(handle)) + .replace(handle, replace(handle)) .expect("Cycle not found"); Region::new(self.exterior().clone(), interiors, self.color()) } diff --git a/crates/fj-core/src/operations/update/shell.rs b/crates/fj-core/src/operations/update/shell.rs index 1216a6b89..e39fa92a5 100644 --- a/crates/fj-core/src/operations/update/shell.rs +++ b/crates/fj-core/src/operations/update/shell.rs @@ -58,7 +58,7 @@ impl UpdateShell for Shell { ) -> Self { let faces = self .faces() - .replace(handle, update(handle)) + .replace(handle, [update(handle)]) .expect("Face not found"); Shell::new(faces) } @@ -70,7 +70,7 @@ impl UpdateShell for Shell { ) -> Self { let faces = self .faces() - .replace_with_multiple(handle, replace(handle)) + .replace(handle, replace(handle)) .expect("Face not found"); Shell::new(faces) } diff --git a/crates/fj-core/src/operations/update/sketch.rs b/crates/fj-core/src/operations/update/sketch.rs index 3faffcb80..6022cafde 100644 --- a/crates/fj-core/src/operations/update/sketch.rs +++ b/crates/fj-core/src/operations/update/sketch.rs @@ -53,7 +53,7 @@ impl UpdateSketch for Sketch { ) -> Self { let regions = self .regions() - .replace(handle, update(handle)) + .replace(handle, [update(handle)]) .expect("Region not found"); Sketch::new(regions) } @@ -65,7 +65,7 @@ impl UpdateSketch for Sketch { ) -> Self { let regions = self .regions() - .replace_with_multiple(handle, replace(handle)) + .replace(handle, replace(handle)) .expect("Region not found"); Sketch::new(regions) } diff --git a/crates/fj-core/src/operations/update/solid.rs b/crates/fj-core/src/operations/update/solid.rs index 02a44adda..13f8a3748 100644 --- a/crates/fj-core/src/operations/update/solid.rs +++ b/crates/fj-core/src/operations/update/solid.rs @@ -60,7 +60,7 @@ impl UpdateSolid for Solid { ) -> Self { let shells = self .shells() - .replace(handle, update(handle)) + .replace(handle, [update(handle)]) .expect("Shell not found"); Solid::new(shells) } @@ -72,7 +72,7 @@ impl UpdateSolid for Solid { ) -> Self { let shells = self .shells() - .replace_with_multiple(handle, replace(handle)) + .replace(handle, replace(handle)) .expect("Shell not found"); Solid::new(shells) }