From df06c263520bad0799e108049683a6682d71822f Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 11:39:25 +0200
Subject: [PATCH 01/22] Simplify struct name

---
 crates/fj-core/src/objects/handles.rs      | 6 +++---
 crates/fj-core/src/objects/kinds/cycle.rs  | 4 ++--
 crates/fj-core/src/objects/kinds/region.rs | 4 ++--
 crates/fj-core/src/objects/kinds/shell.rs  | 4 ++--
 crates/fj-core/src/objects/kinds/sketch.rs | 4 ++--
 crates/fj-core/src/objects/kinds/solid.rs  | 4 ++--
 6 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/crates/fj-core/src/objects/handles.rs b/crates/fj-core/src/objects/handles.rs
index d6afcdd10..21ce626a8 100644
--- a/crates/fj-core/src/objects/handles.rs
+++ b/crates/fj-core/src/objects/handles.rs
@@ -13,7 +13,7 @@ use crate::storage::Handle;
 /// `HandleSet` implement `FromIterator`, but it must never be constructed from
 /// an iterator that contains duplicate handles. This will result in a panic.
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
-pub struct HandleSet<T> {
+pub struct Handles<T> {
     // This is supposed to be a set data structure, so what is that `Vec` doing
     // here? Well, it's here because we need it to preserve insertion order, but
     // that doesn't explain why it is here *alone*.
@@ -26,7 +26,7 @@ pub struct HandleSet<T> {
     inner: Vec<Handle<T>>,
 }
 
-impl<T> HandleSet<T> {
+impl<T> Handles<T> {
     pub fn len(&self) -> usize {
         self.inner.len()
     }
@@ -43,7 +43,7 @@ impl<T> HandleSet<T> {
     }
 }
 
-impl<O> FromIterator<Handle<O>> for HandleSet<O>
+impl<O> FromIterator<Handle<O>> for Handles<O>
 where
     O: Debug + Ord,
 {
diff --git a/crates/fj-core/src/objects/kinds/cycle.rs b/crates/fj-core/src/objects/kinds/cycle.rs
index ffa3f9a6b..22d1d99fa 100644
--- a/crates/fj-core/src/objects/kinds/cycle.rs
+++ b/crates/fj-core/src/objects/kinds/cycle.rs
@@ -2,14 +2,14 @@ use fj_math::{Scalar, Winding};
 
 use crate::{
     geometry::SurfacePath,
-    objects::{handles::HandleSet, Edge, HandleIter},
+    objects::{handles::Handles, Edge, HandleIter},
     storage::Handle,
 };
 
 /// A cycle of connected edges
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct Cycle {
-    edges: HandleSet<Edge>,
+    edges: Handles<Edge>,
 }
 
 impl Cycle {
diff --git a/crates/fj-core/src/objects/kinds/region.rs b/crates/fj-core/src/objects/kinds/region.rs
index e9028b5e2..e345beb53 100644
--- a/crates/fj-core/src/objects/kinds/region.rs
+++ b/crates/fj-core/src/objects/kinds/region.rs
@@ -2,7 +2,7 @@
 use fj_interop::mesh::Color;
 
 use crate::{
-    objects::{handles::HandleSet, Cycle, HandleIter},
+    objects::{handles::Handles, Cycle, HandleIter},
     storage::Handle,
 };
 
@@ -17,7 +17,7 @@ use crate::{
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct Region {
     exterior: Handle<Cycle>,
-    interiors: HandleSet<Cycle>,
+    interiors: Handles<Cycle>,
     color: Option<Color>,
 }
 
diff --git a/crates/fj-core/src/objects/kinds/shell.rs b/crates/fj-core/src/objects/kinds/shell.rs
index c8654945f..2659eb85a 100644
--- a/crates/fj-core/src/objects/kinds/shell.rs
+++ b/crates/fj-core/src/objects/kinds/shell.rs
@@ -1,12 +1,12 @@
 use crate::{
-    objects::{handles::HandleSet, Face, HandleIter},
+    objects::{handles::Handles, Face, HandleIter},
     storage::Handle,
 };
 
 /// A 3-dimensional closed shell
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct Shell {
-    faces: HandleSet<Face>,
+    faces: Handles<Face>,
 }
 
 impl Shell {
diff --git a/crates/fj-core/src/objects/kinds/sketch.rs b/crates/fj-core/src/objects/kinds/sketch.rs
index 3709783fd..c32f30731 100644
--- a/crates/fj-core/src/objects/kinds/sketch.rs
+++ b/crates/fj-core/src/objects/kinds/sketch.rs
@@ -1,12 +1,12 @@
 use crate::{
-    objects::{handles::HandleSet, HandleIter, Region},
+    objects::{handles::Handles, HandleIter, Region},
     storage::Handle,
 };
 
 /// A 2-dimensional shape
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct Sketch {
-    regions: HandleSet<Region>,
+    regions: Handles<Region>,
 }
 
 impl Sketch {
diff --git a/crates/fj-core/src/objects/kinds/solid.rs b/crates/fj-core/src/objects/kinds/solid.rs
index fe973df9d..5d9b3854c 100644
--- a/crates/fj-core/src/objects/kinds/solid.rs
+++ b/crates/fj-core/src/objects/kinds/solid.rs
@@ -1,5 +1,5 @@
 use crate::{
-    objects::{handles::HandleSet, HandleIter, Shell},
+    objects::{handles::Handles, HandleIter, Shell},
     storage::Handle,
 };
 
@@ -13,7 +13,7 @@ use crate::{
 /// not currently validated.
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct Solid {
-    shells: HandleSet<Shell>,
+    shells: Handles<Shell>,
 }
 
 impl Solid {

From 45f0b97b8eddc7a9d7087cd410e31f72d4d7554c Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 11:41:02 +0200
Subject: [PATCH 02/22] Update documentation of `Handles`

---
 crates/fj-core/src/objects/handles.rs | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/crates/fj-core/src/objects/handles.rs b/crates/fj-core/src/objects/handles.rs
index 21ce626a8..fabc96839 100644
--- a/crates/fj-core/src/objects/handles.rs
+++ b/crates/fj-core/src/objects/handles.rs
@@ -6,9 +6,9 @@ use crate::storage::Handle;
 
 /// An ordered set of object handles
 ///
-/// This is an internal data structure that is used within objects that
-/// reference multiple other objects of the same type. It does not contain any
-/// duplicate elements, and it maintains the insertion order of those elements.
+/// This is the data structure used by all objects that reference multiple
+/// objects of the same type. It is a set, not containing any duplicate
+/// elements, and it maintains the insertion order of those elements.
 ///
 /// `HandleSet` implement `FromIterator`, but it must never be constructed from
 /// an iterator that contains duplicate handles. This will result in a panic.
@@ -27,14 +27,17 @@ pub struct Handles<T> {
 }
 
 impl<T> Handles<T> {
+    /// Return the number of handles in this set
     pub fn len(&self) -> usize {
         self.inner.len()
     }
 
+    /// Indicate whether the set is empty
     pub fn is_empty(&self) -> bool {
         self.inner.is_empty()
     }
 
+    /// Access an iterator over the handles
     pub fn iter(&self) -> HandleIter<T> {
         HandleIter {
             handles: &self.inner,

From b911d0e78fa36b7c5a79e55e3b2a68c64f04ffca Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 11:43:10 +0200
Subject: [PATCH 03/22] Expose `Handles` in the public API

This prepares for a refactoring of the roles of `Handles` and
`HandleIter`
---
 crates/fj-core/src/objects/mod.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crates/fj-core/src/objects/mod.rs b/crates/fj-core/src/objects/mod.rs
index 0f170f05d..53c588eda 100644
--- a/crates/fj-core/src/objects/mod.rs
+++ b/crates/fj-core/src/objects/mod.rs
@@ -46,7 +46,7 @@ mod set;
 mod stores;
 
 pub use self::{
-    handles::HandleIter,
+    handles::{HandleIter, Handles},
     kinds::{
         curve::Curve,
         cycle::Cycle,

From 878ead7d98ea611ad5dded7f6a965d04b3d9c0b7 Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:05:19 +0200
Subject: [PATCH 04/22] Refactor to prepare for follow-on change

---
 crates/fj-core/src/objects/handles.rs | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/crates/fj-core/src/objects/handles.rs b/crates/fj-core/src/objects/handles.rs
index fabc96839..b9f9a4f1a 100644
--- a/crates/fj-core/src/objects/handles.rs
+++ b/crates/fj-core/src/objects/handles.rs
@@ -40,7 +40,7 @@ impl<T> Handles<T> {
     /// Access an iterator over the handles
     pub fn iter(&self) -> HandleIter<T> {
         HandleIter {
-            handles: &self.inner,
+            handles: self,
             next_index: 0,
         }
     }
@@ -75,7 +75,7 @@ where
 /// This struct is returned by the respective methods of all objects that
 /// reference multiple objects of the same type.
 pub struct HandleIter<'r, T> {
-    handles: &'r Vec<Handle<T>>,
+    handles: &'r Handles<T>,
     next_index: usize,
 }
 
@@ -84,7 +84,7 @@ impl<'r, T> HandleIter<'r, T> {
     ///
     /// This method is unaffected by any previous calls to `next`.
     pub fn nth(&self, index: usize) -> Option<&Handle<T>> {
-        self.handles.get(index)
+        self.handles.inner.get(index)
     }
 
     /// Return the n-th item, treating the iterator as circular
@@ -103,7 +103,10 @@ impl<'r, T> HandleIter<'r, T> {
     ///
     /// This method is unaffected by any previous calls to `next`.
     pub fn index_of(&self, handle: &Handle<T>) -> Option<usize> {
-        self.handles.iter().position(|h| h.id() == handle.id())
+        self.handles
+            .inner
+            .iter()
+            .position(|h| h.id() == handle.id())
     }
 
     /// Access the item after the provided one
@@ -132,13 +135,13 @@ impl<'r, T> Iterator for HandleIter<'r, T> {
     type Item = &'r Handle<T>;
 
     fn next(&mut self) -> Option<Self::Item> {
-        let handle = self.handles.get(self.next_index);
+        let handle = self.handles.inner.get(self.next_index);
         self.next_index += 1;
         handle
     }
 
     fn size_hint(&self) -> (usize, Option<usize>) {
-        let size = self.handles.len();
+        let size = self.handles.inner.len();
         (size, Some(size))
     }
 }

From c657f0d07a39a9310c0a639b7b788599391491d3 Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:09:39 +0200
Subject: [PATCH 05/22] Duplicate `HandleIter` API on `Handles`

This is in preparation of removing `HandleIter` and having `Handles`
take over its role.
---
 crates/fj-core/src/objects/handles.rs | 49 +++++++++++++++++++++------
 1 file changed, 38 insertions(+), 11 deletions(-)

diff --git a/crates/fj-core/src/objects/handles.rs b/crates/fj-core/src/objects/handles.rs
index b9f9a4f1a..f5df2fa0f 100644
--- a/crates/fj-core/src/objects/handles.rs
+++ b/crates/fj-core/src/objects/handles.rs
@@ -37,6 +37,34 @@ impl<T> Handles<T> {
         self.inner.is_empty()
     }
 
+    /// Return the n-th item
+    pub fn nth(&self, index: usize) -> Option<&Handle<T>> {
+        self.inner.get(index)
+    }
+
+    /// Return the n-th item, treating the index space as circular
+    ///
+    /// If the length of `Handles` is `i`, then retrieving the i-th edge using
+    /// this method, is the same as retrieving the 0-th one.
+    pub fn nth_circular(&self, index: usize) -> &Handle<T> {
+        let index = index % self.len();
+        self.nth(index)
+            .expect("Index must be valid, due to modulo above")
+    }
+
+    /// Return the index of the item, if available
+    pub fn index_of(&self, handle: &Handle<T>) -> Option<usize> {
+        self.inner.iter().position(|h| h.id() == handle.id())
+    }
+
+    /// Access the item after the provided one
+    ///
+    /// Returns `None`, if the provided item is not in this iterator.
+    pub fn after(&self, handle: &Handle<T>) -> Option<&Handle<T>> {
+        self.index_of(handle)
+            .map(|index| self.nth_circular(index + 1))
+    }
+
     /// Access an iterator over the handles
     pub fn iter(&self) -> HandleIter<T> {
         HandleIter {
@@ -44,6 +72,11 @@ impl<T> Handles<T> {
             next_index: 0,
         }
     }
+
+    /// Return iterator over the pairs of all handles
+    pub fn pairs(&self) -> impl Iterator<Item = (&Handle<T>, &Handle<T>)> {
+        self.iter().circular_tuple_windows()
+    }
 }
 
 impl<O> FromIterator<Handle<O>> for Handles<O>
@@ -84,7 +117,7 @@ impl<'r, T> HandleIter<'r, T> {
     ///
     /// This method is unaffected by any previous calls to `next`.
     pub fn nth(&self, index: usize) -> Option<&Handle<T>> {
-        self.handles.inner.get(index)
+        self.handles.nth(index)
     }
 
     /// Return the n-th item, treating the iterator as circular
@@ -94,32 +127,26 @@ impl<'r, T> HandleIter<'r, T> {
     ///
     /// This method is unaffected by any previous calls to `next`.
     pub fn nth_circular(&self, index: usize) -> &Handle<T> {
-        let index = index % self.len();
-        self.nth(index)
-            .expect("Index must be valid, due to modulo above")
+        self.handles.nth_circular(index)
     }
 
     /// Return the index of the item, if it is in this iterator
     ///
     /// This method is unaffected by any previous calls to `next`.
     pub fn index_of(&self, handle: &Handle<T>) -> Option<usize> {
-        self.handles
-            .inner
-            .iter()
-            .position(|h| h.id() == handle.id())
+        self.handles.index_of(handle)
     }
 
     /// Access the item after the provided one
     ///
     /// Returns `None`, if the provided item is not in this iterator.
     pub fn after(&self, handle: &Handle<T>) -> Option<&Handle<T>> {
-        self.index_of(handle)
-            .map(|index| self.nth_circular(index + 1))
+        self.handles.after(handle)
     }
 
     /// Return iterator over the pairs of the remaining items in this iterator
     pub fn pairs(self) -> impl Iterator<Item = (&'r Handle<T>, &'r Handle<T>)> {
-        self.circular_tuple_windows()
+        self.handles.pairs()
     }
 }
 

From 77eee8f055b159fe7a8385d19ae018763cfab23f Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:12:36 +0200
Subject: [PATCH 06/22] Add `Handles::new`

---
 crates/fj-core/src/objects/handles.rs | 44 +++++++++++++++++----------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/crates/fj-core/src/objects/handles.rs b/crates/fj-core/src/objects/handles.rs
index f5df2fa0f..2e44b3768 100644
--- a/crates/fj-core/src/objects/handles.rs
+++ b/crates/fj-core/src/objects/handles.rs
@@ -27,6 +27,33 @@ pub struct Handles<T> {
 }
 
 impl<T> Handles<T> {
+    /// Create a new instances of `Handles` from an iterator over `Handle<T>`
+    ///
+    /// # Panics
+    ///
+    /// Panics, if the iterator contains duplicate `Handle`s.
+    pub fn new(handles: impl IntoIterator<Item = Handle<T>>) -> Self
+    where
+        T: Debug + Ord,
+    {
+        let mut added = BTreeSet::new();
+        let mut inner = Vec::new();
+
+        for handle in handles {
+            if added.contains(&handle) {
+                panic!(
+                    "Constructing `HandleSet` with duplicate handle: {:?}",
+                    handle
+                );
+            }
+
+            added.insert(handle.clone());
+            inner.push(handle);
+        }
+
+        Self { inner }
+    }
+
     /// Return the number of handles in this set
     pub fn len(&self) -> usize {
         self.inner.len()
@@ -84,22 +111,7 @@ where
     O: Debug + Ord,
 {
     fn from_iter<T: IntoIterator<Item = Handle<O>>>(handles: T) -> Self {
-        let mut added = BTreeSet::new();
-        let mut inner = Vec::new();
-
-        for handle in handles {
-            if added.contains(&handle) {
-                panic!(
-                    "Constructing `HandleSet` with duplicate handle: {:?}",
-                    handle
-                );
-            }
-
-            added.insert(handle.clone());
-            inner.push(handle);
-        }
-
-        Self { inner }
+        Self::new(handles)
     }
 }
 

From 3f05de778bd52b699b3ea8dd05ff8f2b292a8b8c Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:12:55 +0200
Subject: [PATCH 07/22] Remove redundant paragraph from doc comment

---
 crates/fj-core/src/objects/handles.rs | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/crates/fj-core/src/objects/handles.rs b/crates/fj-core/src/objects/handles.rs
index 2e44b3768..ddf4fe4d0 100644
--- a/crates/fj-core/src/objects/handles.rs
+++ b/crates/fj-core/src/objects/handles.rs
@@ -9,9 +9,6 @@ use crate::storage::Handle;
 /// This is the data structure used by all objects that reference multiple
 /// objects of the same type. It is a set, not containing any duplicate
 /// elements, and it maintains the insertion order of those elements.
-///
-/// `HandleSet` implement `FromIterator`, but it must never be constructed from
-/// an iterator that contains duplicate handles. This will result in a panic.
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct Handles<T> {
     // This is supposed to be a set data structure, so what is that `Vec` doing

From af1670eb240f6ab8150ca321e186076227024e8b Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:16:44 +0200
Subject: [PATCH 08/22] Implement `IntoIterator` for `Handles`

---
 crates/fj-core/src/objects/handles.rs | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/crates/fj-core/src/objects/handles.rs b/crates/fj-core/src/objects/handles.rs
index ddf4fe4d0..63fbd1f13 100644
--- a/crates/fj-core/src/objects/handles.rs
+++ b/crates/fj-core/src/objects/handles.rs
@@ -112,6 +112,15 @@ where
     }
 }
 
+impl<'r, T> IntoIterator for &'r Handles<T> {
+    type Item = &'r Handle<T>;
+    type IntoIter = HandleIter<'r, T>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.iter()
+    }
+}
+
 /// An iterator over handles to objects
 ///
 /// This struct is returned by the respective methods of all objects that

From 8711e8f9efc60e3c393ae1fddcde4a152df9366a Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:17:21 +0200
Subject: [PATCH 09/22] Return `&Handles` from `Solid::shells`

---
 crates/fj-core/src/algorithms/approx/solid.rs    | 1 +
 crates/fj-core/src/algorithms/transform/solid.rs | 2 +-
 crates/fj-core/src/objects/kinds/solid.rs        | 6 +++---
 crates/fj-core/src/operations/merge.rs           | 2 +-
 crates/fj-core/src/operations/update/solid.rs    | 2 +-
 crates/fj-core/src/validate/solid.rs             | 1 +
 6 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/crates/fj-core/src/algorithms/approx/solid.rs b/crates/fj-core/src/algorithms/approx/solid.rs
index 8fae93993..97ad09fc1 100644
--- a/crates/fj-core/src/algorithms/approx/solid.rs
+++ b/crates/fj-core/src/algorithms/approx/solid.rs
@@ -18,6 +18,7 @@ impl Approx for &Solid {
         let tolerance = tolerance.into();
 
         self.shells()
+            .iter()
             .flat_map(|shell| shell.approx_with_cache(tolerance, cache))
             .collect()
     }
diff --git a/crates/fj-core/src/algorithms/transform/solid.rs b/crates/fj-core/src/algorithms/transform/solid.rs
index fa9c243a5..976b6bed2 100644
--- a/crates/fj-core/src/algorithms/transform/solid.rs
+++ b/crates/fj-core/src/algorithms/transform/solid.rs
@@ -11,7 +11,7 @@ impl TransformObject for Solid {
         services: &mut Services,
         cache: &mut TransformCache,
     ) -> Self {
-        let shells = self.shells().cloned().map(|shell| {
+        let shells = self.shells().iter().cloned().map(|shell| {
             shell.transform_with_cache(transform, services, cache)
         });
 
diff --git a/crates/fj-core/src/objects/kinds/solid.rs b/crates/fj-core/src/objects/kinds/solid.rs
index 5d9b3854c..203ca6189 100644
--- a/crates/fj-core/src/objects/kinds/solid.rs
+++ b/crates/fj-core/src/objects/kinds/solid.rs
@@ -1,5 +1,5 @@
 use crate::{
-    objects::{handles::Handles, HandleIter, Shell},
+    objects::{handles::Handles, Shell},
     storage::Handle,
 };
 
@@ -25,7 +25,7 @@ impl Solid {
     }
 
     /// Access the solid's shells
-    pub fn shells(&self) -> HandleIter<Shell> {
-        self.shells.iter()
+    pub fn shells(&self) -> &Handles<Shell> {
+        &self.shells
     }
 }
diff --git a/crates/fj-core/src/operations/merge.rs b/crates/fj-core/src/operations/merge.rs
index 067a10c84..201b89154 100644
--- a/crates/fj-core/src/operations/merge.rs
+++ b/crates/fj-core/src/operations/merge.rs
@@ -11,6 +11,6 @@ pub trait Merge {
 
 impl Merge for Solid {
     fn merge(&self, other: &Self) -> Self {
-        self.add_shells(other.shells().cloned())
+        self.add_shells(other.shells().iter().cloned())
     }
 }
diff --git a/crates/fj-core/src/operations/update/solid.rs b/crates/fj-core/src/operations/update/solid.rs
index a1460cb30..21f943fe8 100644
--- a/crates/fj-core/src/operations/update/solid.rs
+++ b/crates/fj-core/src/operations/update/solid.rs
@@ -18,7 +18,7 @@ impl UpdateSolid for Solid {
         &self,
         shells: impl IntoIterator<Item = Handle<Shell>>,
     ) -> Self {
-        let shells = self.shells().cloned().chain(shells);
+        let shells = self.shells().iter().cloned().chain(shells);
         Solid::new(shells)
     }
 }
diff --git a/crates/fj-core/src/validate/solid.rs b/crates/fj-core/src/validate/solid.rs
index 550dce114..369b96426 100644
--- a/crates/fj-core/src/validate/solid.rs
+++ b/crates/fj-core/src/validate/solid.rs
@@ -70,6 +70,7 @@ impl SolidValidationError {
     ) {
         let vertices: Vec<(Point<3>, Handle<Vertex>)> = solid
             .shells()
+            .iter()
             .flat_map(|s| s.faces())
             .flat_map(|face| {
                 face.region()

From 2fc7b65510b0428b9bfb8a03d73ba8493c477dab Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:18:52 +0200
Subject: [PATCH 10/22] Return `&Handles` from `Shell::faces`

---
 crates/fj-core/src/algorithms/approx/shell.rs    | 2 +-
 crates/fj-core/src/algorithms/transform/shell.rs | 8 ++++----
 crates/fj-core/src/objects/kinds/shell.rs        | 6 +++---
 crates/fj-core/src/operations/update/shell.rs    | 5 +++--
 4 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/crates/fj-core/src/algorithms/approx/shell.rs b/crates/fj-core/src/algorithms/approx/shell.rs
index 1fbb21121..0bbef83f9 100644
--- a/crates/fj-core/src/algorithms/approx/shell.rs
+++ b/crates/fj-core/src/algorithms/approx/shell.rs
@@ -15,6 +15,6 @@ impl Approx for &Shell {
         tolerance: impl Into<Tolerance>,
         cache: &mut Self::Cache,
     ) -> Self::Approximation {
-        self.faces().approx_with_cache(tolerance, cache)
+        self.faces().iter().approx_with_cache(tolerance, cache)
     }
 }
diff --git a/crates/fj-core/src/algorithms/transform/shell.rs b/crates/fj-core/src/algorithms/transform/shell.rs
index 0d1d474e0..835299209 100644
--- a/crates/fj-core/src/algorithms/transform/shell.rs
+++ b/crates/fj-core/src/algorithms/transform/shell.rs
@@ -11,10 +11,10 @@ impl TransformObject for Shell {
         services: &mut Services,
         cache: &mut TransformCache,
     ) -> Self {
-        let faces = self
-            .faces()
-            .cloned()
-            .map(|face| face.transform_with_cache(transform, services, cache));
+        let faces =
+            self.faces().iter().cloned().map(|face| {
+                face.transform_with_cache(transform, services, cache)
+            });
 
         Self::new(faces)
     }
diff --git a/crates/fj-core/src/objects/kinds/shell.rs b/crates/fj-core/src/objects/kinds/shell.rs
index 2659eb85a..c905cf70b 100644
--- a/crates/fj-core/src/objects/kinds/shell.rs
+++ b/crates/fj-core/src/objects/kinds/shell.rs
@@ -1,5 +1,5 @@
 use crate::{
-    objects::{handles::Handles, Face, HandleIter},
+    objects::{handles::Handles, Face},
     storage::Handle,
 };
 
@@ -18,7 +18,7 @@ impl Shell {
     }
 
     /// Access the faces of the shell
-    pub fn faces(&self) -> HandleIter<Face> {
-        self.faces.iter()
+    pub fn faces(&self) -> &Handles<Face> {
+        &self.faces
     }
 }
diff --git a/crates/fj-core/src/operations/update/shell.rs b/crates/fj-core/src/operations/update/shell.rs
index 52c9c6f36..eb7d78928 100644
--- a/crates/fj-core/src/operations/update/shell.rs
+++ b/crates/fj-core/src/operations/update/shell.rs
@@ -24,7 +24,7 @@ pub trait UpdateShell {
 
 impl UpdateShell for Shell {
     fn add_faces(&self, faces: impl IntoIterator<Item = Handle<Face>>) -> Self {
-        let faces = self.faces().cloned().chain(faces);
+        let faces = self.faces().iter().cloned().chain(faces);
         Shell::new(faces)
     }
 
@@ -33,7 +33,7 @@ impl UpdateShell for Shell {
         original: &Handle<Face>,
         replacement: Handle<Face>,
     ) -> Self {
-        let faces = self.faces().map(|face| {
+        let faces = self.faces().iter().map(|face| {
             if face.id() == original.id() {
                 replacement.clone()
             } else {
@@ -47,6 +47,7 @@ impl UpdateShell for Shell {
     fn remove_face(&self, handle: &Handle<Face>) -> Self {
         let faces = self
             .faces()
+            .iter()
             .filter(|face| face.id() == handle.id())
             .cloned();
 

From 620cd44dba3f70c0e639695a6b724a358fd469d5 Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:20:01 +0200
Subject: [PATCH 11/22] Return `&Handles` from `Sketch::regions`

---
 crates/fj-core/src/objects/kinds/sketch.rs     | 6 +++---
 crates/fj-core/src/operations/update/sketch.rs | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/crates/fj-core/src/objects/kinds/sketch.rs b/crates/fj-core/src/objects/kinds/sketch.rs
index c32f30731..f6d19d22b 100644
--- a/crates/fj-core/src/objects/kinds/sketch.rs
+++ b/crates/fj-core/src/objects/kinds/sketch.rs
@@ -1,5 +1,5 @@
 use crate::{
-    objects::{handles::Handles, HandleIter, Region},
+    objects::{handles::Handles, Region},
     storage::Handle,
 };
 
@@ -18,7 +18,7 @@ impl Sketch {
     }
 
     /// Access the regions of the sketch
-    pub fn regions(&self) -> HandleIter<Region> {
-        self.regions.iter()
+    pub fn regions(&self) -> &Handles<Region> {
+        &self.regions
     }
 }
diff --git a/crates/fj-core/src/operations/update/sketch.rs b/crates/fj-core/src/operations/update/sketch.rs
index 87739942a..d81c1f528 100644
--- a/crates/fj-core/src/operations/update/sketch.rs
+++ b/crates/fj-core/src/operations/update/sketch.rs
@@ -12,6 +12,6 @@ pub trait UpdateSketch {
 
 impl UpdateSketch for Sketch {
     fn add_region(&self, region: Handle<Region>) -> Self {
-        Sketch::new(self.regions().cloned().chain([region]))
+        Sketch::new(self.regions().iter().cloned().chain([region]))
     }
 }

From dd7a3ecd90d13090c555023ac069b556b8e0d9c9 Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:21:58 +0200
Subject: [PATCH 12/22] Return `&Handles` from `Region::interiors`

---
 crates/fj-core/src/algorithms/transform/face.rs | 7 ++++---
 crates/fj-core/src/objects/kinds/region.rs      | 6 +++---
 crates/fj-core/src/operations/reverse/region.rs | 3 ++-
 crates/fj-core/src/operations/update/region.rs  | 4 ++--
 crates/fj-core/src/validate/face.rs             | 1 +
 5 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/crates/fj-core/src/algorithms/transform/face.rs b/crates/fj-core/src/algorithms/transform/face.rs
index 1aa6e2278..a721e9a21 100644
--- a/crates/fj-core/src/algorithms/transform/face.rs
+++ b/crates/fj-core/src/algorithms/transform/face.rs
@@ -27,9 +27,10 @@ impl TransformObject for Face {
             .exterior()
             .clone()
             .transform_with_cache(transform, services, cache);
-        let interiors = self.region().interiors().cloned().map(|interior| {
-            interior.transform_with_cache(transform, services, cache)
-        });
+        let interiors =
+            self.region().interiors().iter().cloned().map(|interior| {
+                interior.transform_with_cache(transform, services, cache)
+            });
 
         let region = Region::new(exterior, interiors, color).insert(services);
 
diff --git a/crates/fj-core/src/objects/kinds/region.rs b/crates/fj-core/src/objects/kinds/region.rs
index e345beb53..c1fa01807 100644
--- a/crates/fj-core/src/objects/kinds/region.rs
+++ b/crates/fj-core/src/objects/kinds/region.rs
@@ -2,7 +2,7 @@
 use fj_interop::mesh::Color;
 
 use crate::{
-    objects::{handles::Handles, Cycle, HandleIter},
+    objects::{handles::Handles, Cycle},
     storage::Handle,
 };
 
@@ -43,8 +43,8 @@ impl Region {
     /// Access the cycles that bound the region on the inside
     ///
     /// Each of these cycles defines a hole in the region .
-    pub fn interiors(&self) -> HandleIter<Cycle> {
-        self.interiors.iter()
+    pub fn interiors(&self) -> &Handles<Cycle> {
+        &self.interiors
     }
 
     /// Access all cycles of the region (both exterior and interior)
diff --git a/crates/fj-core/src/operations/reverse/region.rs b/crates/fj-core/src/operations/reverse/region.rs
index 9c1c3026f..554f85b10 100644
--- a/crates/fj-core/src/operations/reverse/region.rs
+++ b/crates/fj-core/src/operations/reverse/region.rs
@@ -7,6 +7,7 @@ impl Reverse for Region {
         let exterior = self.exterior().reverse(services).insert(services);
         let interiors = self
             .interiors()
+            .iter()
             .map(|cycle| cycle.reverse(services).insert(services));
 
         Region::new(exterior, interiors, self.color())
@@ -22,7 +23,7 @@ impl ReverseCurveCoordinateSystems for Region {
             .exterior()
             .reverse_curve_coordinate_systems(services)
             .insert(services);
-        let interiors = self.interiors().map(|cycle| {
+        let interiors = self.interiors().iter().map(|cycle| {
             cycle
                 .reverse_curve_coordinate_systems(services)
                 .insert(services)
diff --git a/crates/fj-core/src/operations/update/region.rs b/crates/fj-core/src/operations/update/region.rs
index 7fe62e6f2..f32fd4b23 100644
--- a/crates/fj-core/src/operations/update/region.rs
+++ b/crates/fj-core/src/operations/update/region.rs
@@ -26,14 +26,14 @@ impl UpdateRegion for Region {
         f: impl FnOnce(&Handle<Cycle>) -> Handle<Cycle>,
     ) -> Self {
         let exterior = f(self.exterior());
-        Region::new(exterior, self.interiors().cloned(), self.color())
+        Region::new(exterior, self.interiors().iter().cloned(), self.color())
     }
 
     fn add_interiors(
         &self,
         interiors: impl IntoIterator<Item = Handle<Cycle>>,
     ) -> Self {
-        let interiors = self.interiors().cloned().chain(interiors);
+        let interiors = self.interiors().iter().cloned().chain(interiors);
         Region::new(self.exterior().clone(), interiors, self.color())
     }
 }
diff --git a/crates/fj-core/src/validate/face.rs b/crates/fj-core/src/validate/face.rs
index 7c336d199..cb7e86438 100644
--- a/crates/fj-core/src/validate/face.rs
+++ b/crates/fj-core/src/validate/face.rs
@@ -106,6 +106,7 @@ mod tests {
             let interiors = valid
                 .region()
                 .interiors()
+                .iter()
                 .cloned()
                 .map(|cycle| cycle.reverse(&mut services).insert(&mut services))
                 .collect::<Vec<_>>();

From 89227311b4696442b56028e3c9b8f5804704ccf7 Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:23:05 +0200
Subject: [PATCH 13/22] Update comment

---
 crates/fj-core/src/objects/kinds/region.rs | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/crates/fj-core/src/objects/kinds/region.rs b/crates/fj-core/src/objects/kinds/region.rs
index c1fa01807..a68d06594 100644
--- a/crates/fj-core/src/objects/kinds/region.rs
+++ b/crates/fj-core/src/objects/kinds/region.rs
@@ -49,11 +49,8 @@ impl Region {
 
     /// Access all cycles of the region (both exterior and interior)
     pub fn all_cycles(&self) -> impl Iterator<Item = &Handle<Cycle>> {
-        // It would be nice to return `HandleIter` here, but there's no
-        // straight-forward way to chain it to another iterator, given it's
-        // current implementation.
-        //
-        // Maybe this can be addressed, once the need arises.
+        // It would be nice to return `&Handles` here, but I don't see a way for
+        // doing that here *and* in `interiors`.
         [self.exterior()].into_iter().chain(self.interiors())
     }
 

From ff3ff4046769358826d9ccb749e064141319ba87 Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:25:37 +0200
Subject: [PATCH 14/22] Return `&Handles` from `Cycle::edges`

---
 crates/fj-core/src/algorithms/approx/cycle.rs         | 1 +
 crates/fj-core/src/algorithms/intersect/face_point.rs | 3 +++
 crates/fj-core/src/algorithms/intersect/ray_face.rs   | 2 ++
 crates/fj-core/src/algorithms/transform/cycle.rs      | 2 +-
 crates/fj-core/src/objects/kinds/cycle.rs             | 7 ++++---
 crates/fj-core/src/operations/build/face.rs           | 2 +-
 crates/fj-core/src/operations/reverse/cycle.rs        | 2 +-
 crates/fj-core/src/operations/update/cycle.rs         | 6 +++---
 crates/fj-core/src/queries/all_edges_with_surface.rs  | 1 +
 crates/fj-core/src/validate/cycle.rs                  | 2 +-
 crates/fj-core/src/validate/face.rs                   | 4 ++--
 crates/fj-core/src/validate/solid.rs                  | 2 +-
 12 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/crates/fj-core/src/algorithms/approx/cycle.rs b/crates/fj-core/src/algorithms/approx/cycle.rs
index 998ac685c..94c3e9564 100644
--- a/crates/fj-core/src/algorithms/approx/cycle.rs
+++ b/crates/fj-core/src/algorithms/approx/cycle.rs
@@ -27,6 +27,7 @@ impl Approx for (&Cycle, &Surface) {
 
         let edges = cycle
             .edges()
+            .iter()
             .map(|edge| {
                 (edge.deref(), surface).approx_with_cache(tolerance, cache)
             })
diff --git a/crates/fj-core/src/algorithms/intersect/face_point.rs b/crates/fj-core/src/algorithms/intersect/face_point.rs
index a6f26b1d3..6d09bb6f7 100644
--- a/crates/fj-core/src/algorithms/intersect/face_point.rs
+++ b/crates/fj-core/src/algorithms/intersect/face_point.rs
@@ -29,6 +29,7 @@ impl Intersect for (&Face, &Point<2>) {
             // result of the last segment.
             let mut previous_hit = cycle
                 .edges()
+                .iter()
                 .last()
                 .cloned()
                 .and_then(|edge| (&ray, &edge).intersect());
@@ -336,6 +337,7 @@ mod tests {
             .region()
             .exterior()
             .edges()
+            .iter()
             .find(|edge| edge.start_position() == Point::from([0., 0.]))
             .unwrap();
         assert_eq!(
@@ -371,6 +373,7 @@ mod tests {
             .region()
             .exterior()
             .edges()
+            .iter()
             .find(|edge| edge.start_position() == Point::from([1., 0.]))
             .map(|edge| edge.start_position())
             .unwrap();
diff --git a/crates/fj-core/src/algorithms/intersect/ray_face.rs b/crates/fj-core/src/algorithms/intersect/ray_face.rs
index e5930eed2..164c0cb8b 100644
--- a/crates/fj-core/src/algorithms/intersect/ray_face.rs
+++ b/crates/fj-core/src/algorithms/intersect/ray_face.rs
@@ -263,6 +263,7 @@ mod tests {
             .region()
             .exterior()
             .edges()
+            .iter()
             .find(|edge| edge.start_position() == Point::from([-1., 1.]))
             .unwrap();
         assert_eq!(
@@ -298,6 +299,7 @@ mod tests {
             .region()
             .exterior()
             .edges()
+            .iter()
             .find(|edge| edge.start_position() == Point::from([-1., -1.]))
             .map(|edge| edge.start_position())
             .unwrap();
diff --git a/crates/fj-core/src/algorithms/transform/cycle.rs b/crates/fj-core/src/algorithms/transform/cycle.rs
index 8a6d2ff51..98241f52b 100644
--- a/crates/fj-core/src/algorithms/transform/cycle.rs
+++ b/crates/fj-core/src/algorithms/transform/cycle.rs
@@ -11,7 +11,7 @@ impl TransformObject for Cycle {
         services: &mut Services,
         cache: &mut TransformCache,
     ) -> Self {
-        let edges = self.edges().map(|edge| {
+        let edges = self.edges().iter().map(|edge| {
             edge.clone()
                 .transform_with_cache(transform, services, cache)
         });
diff --git a/crates/fj-core/src/objects/kinds/cycle.rs b/crates/fj-core/src/objects/kinds/cycle.rs
index 22d1d99fa..5c61c2137 100644
--- a/crates/fj-core/src/objects/kinds/cycle.rs
+++ b/crates/fj-core/src/objects/kinds/cycle.rs
@@ -2,7 +2,7 @@ use fj_math::{Scalar, Winding};
 
 use crate::{
     geometry::SurfacePath,
-    objects::{handles::Handles, Edge, HandleIter},
+    objects::{handles::Handles, Edge},
     storage::Handle,
 };
 
@@ -20,8 +20,8 @@ impl Cycle {
     }
 
     /// Access the edges that make up the cycle
-    pub fn edges(&self) -> HandleIter<Edge> {
-        self.edges.iter()
+    pub fn edges(&self) -> &Handles<Edge> {
+        &self.edges
     }
 
     /// Return the number of edges in the cycle
@@ -46,6 +46,7 @@ impl Cycle {
         if self.edges.len() < 3 {
             let first = self
                 .edges()
+                .iter()
                 .next()
                 .expect("Invalid cycle: expected at least one edge");
 
diff --git a/crates/fj-core/src/operations/build/face.rs b/crates/fj-core/src/operations/build/face.rs
index 86157621f..361638565 100644
--- a/crates/fj-core/src/operations/build/face.rs
+++ b/crates/fj-core/src/operations/build/face.rs
@@ -32,7 +32,7 @@ pub trait BuildFace {
         let face = Face::polygon(surface, points_surface, services);
 
         let edges = {
-            let mut edges = face.region().exterior().edges().cloned();
+            let mut edges = face.region().exterior().edges().iter().cloned();
 
             let array = array::from_fn(|_| edges.next()).map(|edge| {
                 edge.expect("Just asserted that there are three edges")
diff --git a/crates/fj-core/src/operations/reverse/cycle.rs b/crates/fj-core/src/operations/reverse/cycle.rs
index 30153a606..5e569163a 100644
--- a/crates/fj-core/src/operations/reverse/cycle.rs
+++ b/crates/fj-core/src/operations/reverse/cycle.rs
@@ -33,7 +33,7 @@ impl ReverseCurveCoordinateSystems for Cycle {
         &self,
         services: &mut Services,
     ) -> Self {
-        let edges = self.edges().map(|edge| {
+        let edges = self.edges().iter().map(|edge| {
             edge.reverse_curve_coordinate_systems(services)
                 .insert(services)
         });
diff --git a/crates/fj-core/src/operations/update/cycle.rs b/crates/fj-core/src/operations/update/cycle.rs
index 4d23ab6d7..beff12c6c 100644
--- a/crates/fj-core/src/operations/update/cycle.rs
+++ b/crates/fj-core/src/operations/update/cycle.rs
@@ -36,7 +36,7 @@ pub trait UpdateCycle {
 
 impl UpdateCycle for Cycle {
     fn add_edges(&self, edges: impl IntoIterator<Item = Handle<Edge>>) -> Self {
-        let edges = self.edges().cloned().chain(edges);
+        let edges = self.edges().iter().cloned().chain(edges);
         Cycle::new(edges)
     }
 
@@ -47,7 +47,7 @@ impl UpdateCycle for Cycle {
     ) -> Self {
         let mut num_replacements = 0;
 
-        let edges = self.edges().map(|edge| {
+        let edges = self.edges().iter().map(|edge| {
             if edge.id() == original.id() {
                 num_replacements += 1;
                 replacement.clone()
@@ -73,7 +73,7 @@ impl UpdateCycle for Cycle {
     ) -> Self {
         let mut num_replacements = 0;
 
-        let edges = self.edges().enumerate().map(|(i, edge)| {
+        let edges = self.edges().iter().enumerate().map(|(i, edge)| {
             if i == index {
                 num_replacements += 1;
                 f(edge)
diff --git a/crates/fj-core/src/queries/all_edges_with_surface.rs b/crates/fj-core/src/queries/all_edges_with_surface.rs
index 809112594..f77dd4424 100644
--- a/crates/fj-core/src/queries/all_edges_with_surface.rs
+++ b/crates/fj-core/src/queries/all_edges_with_surface.rs
@@ -21,6 +21,7 @@ impl AllEdgesWithSurface for Face {
             result.extend(
                 cycle
                     .edges()
+                    .iter()
                     .cloned()
                     .map(|edge| (edge, self.surface().clone())),
             );
diff --git a/crates/fj-core/src/validate/cycle.rs b/crates/fj-core/src/validate/cycle.rs
index e2a63bc6e..81a00b817 100644
--- a/crates/fj-core/src/validate/cycle.rs
+++ b/crates/fj-core/src/validate/cycle.rs
@@ -51,7 +51,7 @@ impl CycleValidationError {
         errors: &mut Vec<ValidationError>,
     ) {
         // If there are no half edges
-        if cycle.edges().next().is_none() {
+        if cycle.edges().iter().next().is_none() {
             errors.push(Self::NotEnoughEdges.into());
         }
     }
diff --git a/crates/fj-core/src/validate/face.rs b/crates/fj-core/src/validate/face.rs
index cb7e86438..ed5c33636 100644
--- a/crates/fj-core/src/validate/face.rs
+++ b/crates/fj-core/src/validate/face.rs
@@ -38,7 +38,7 @@ pub enum FaceValidationError {
 
 impl FaceValidationError {
     fn check_interior_winding(face: &Face, errors: &mut Vec<ValidationError>) {
-        if face.region().exterior().edges().count() == 0 {
+        if face.region().exterior().edges().iter().count() == 0 {
             // Can't determine winding, if the cycle has no edges. Sounds like a
             // job for a different validation check.
             return;
@@ -47,7 +47,7 @@ impl FaceValidationError {
         let exterior_winding = face.region().exterior().winding();
 
         for interior in face.region().interiors() {
-            if interior.edges().count() == 0 {
+            if interior.edges().iter().count() == 0 {
                 // Can't determine winding, if the cycle has no edges. Sounds
                 // like a job for a different validation check.
                 continue;
diff --git a/crates/fj-core/src/validate/solid.rs b/crates/fj-core/src/validate/solid.rs
index 369b96426..4c11a405b 100644
--- a/crates/fj-core/src/validate/solid.rs
+++ b/crates/fj-core/src/validate/solid.rs
@@ -75,7 +75,7 @@ impl SolidValidationError {
             .flat_map(|face| {
                 face.region()
                     .all_cycles()
-                    .flat_map(|cycle| cycle.edges().cloned())
+                    .flat_map(|cycle| cycle.edges().iter().cloned())
                     .zip(repeat(face.surface().geometry()))
             })
             .map(|(h, s)| {

From 9f8ef2651257926f823cb0f54bf975b76d17759a Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:26:51 +0200
Subject: [PATCH 15/22] Refactor to simplify

---
 crates/fj-core/src/validate/face.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crates/fj-core/src/validate/face.rs b/crates/fj-core/src/validate/face.rs
index ed5c33636..da834c3d8 100644
--- a/crates/fj-core/src/validate/face.rs
+++ b/crates/fj-core/src/validate/face.rs
@@ -38,7 +38,7 @@ pub enum FaceValidationError {
 
 impl FaceValidationError {
     fn check_interior_winding(face: &Face, errors: &mut Vec<ValidationError>) {
-        if face.region().exterior().edges().iter().count() == 0 {
+        if face.region().exterior().edges().is_empty() {
             // Can't determine winding, if the cycle has no edges. Sounds like a
             // job for a different validation check.
             return;
@@ -47,7 +47,7 @@ impl FaceValidationError {
         let exterior_winding = face.region().exterior().winding();
 
         for interior in face.region().interiors() {
-            if interior.edges().iter().count() == 0 {
+            if interior.edges().is_empty() {
                 // Can't determine winding, if the cycle has no edges. Sounds
                 // like a job for a different validation check.
                 continue;

From 4751755daaf53e38bffb9a37fe7fa750901d1f99 Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:28:04 +0200
Subject: [PATCH 16/22] Remove redundant methods

---
 crates/fj-core/src/objects/kinds/cycle.rs | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/crates/fj-core/src/objects/kinds/cycle.rs b/crates/fj-core/src/objects/kinds/cycle.rs
index 5c61c2137..e2e47bbff 100644
--- a/crates/fj-core/src/objects/kinds/cycle.rs
+++ b/crates/fj-core/src/objects/kinds/cycle.rs
@@ -24,16 +24,6 @@ impl Cycle {
         &self.edges
     }
 
-    /// Return the number of edges in the cycle
-    pub fn len(&self) -> usize {
-        self.edges.len()
-    }
-
-    /// Indicate whether the cycle is empty
-    pub fn is_empty(&self) -> bool {
-        self.edges.is_empty()
-    }
-
     /// Indicate the cycle's winding, assuming a right-handed coordinate system
     ///
     /// Please note that this is not *the* winding of the cycle, only one of the

From 32e53291875266c213e5107b9c69bf87bd646b03 Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:29:10 +0200
Subject: [PATCH 17/22] Inline redundant variables

This has become possible thanks to the recent shift from `HandleIter` to
`Handles`.
---
 crates/fj-core/src/operations/join/cycle.rs | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/crates/fj-core/src/operations/join/cycle.rs b/crates/fj-core/src/operations/join/cycle.rs
index 7dae78e0f..7fbdaad84 100644
--- a/crates/fj-core/src/operations/join/cycle.rs
+++ b/crates/fj-core/src/operations/join/cycle.rs
@@ -107,20 +107,19 @@ impl JoinCycle for Cycle {
         let mut cycle = self.clone();
 
         for (index, index_other) in range.zip(range_other) {
-            let edges_self = self.edges();
-            let edges_other = other.edges();
+            let edge = self.edges().nth_circular(index);
+            let edge_other = other.edges().nth_circular(index_other);
 
-            let edge = edges_self.nth_circular(index);
-            let edge_other = edges_other.nth_circular(index_other);
-
-            let vertex_a = edges_other
+            let vertex_a = other
+                .edges()
                 .after(edge_other)
                 .expect("Cycle must contain edge; just obtained edge from it")
                 .start_vertex()
                 .clone();
             let vertex_b = edge_other.start_vertex().clone();
 
-            let next_edge = edges_self
+            let next_edge = self
+                .edges()
                 .after(edge)
                 .expect("Cycle must contain edge; just obtained edge from it");
 

From 801d67a53051b3b16a886fe7924674edad1693ba Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:30:45 +0200
Subject: [PATCH 18/22] Replace use of `HandleIter`

---
 crates/fj-core/src/algorithms/approx/face.rs  | 4 ++--
 crates/fj-core/src/algorithms/approx/shell.rs | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/crates/fj-core/src/algorithms/approx/face.rs b/crates/fj-core/src/algorithms/approx/face.rs
index 3babf79d3..9df592ee6 100644
--- a/crates/fj-core/src/algorithms/approx/face.rs
+++ b/crates/fj-core/src/algorithms/approx/face.rs
@@ -7,7 +7,7 @@ use std::{collections::BTreeSet, ops::Deref};
 use fj_interop::mesh::Color;
 
 use crate::{
-    objects::{Face, Handedness, HandleIter},
+    objects::{Face, Handedness, Handles},
     validate::ValidationConfig,
 };
 
@@ -15,7 +15,7 @@ use super::{
     cycle::CycleApprox, edge::EdgeApproxCache, Approx, ApproxPoint, Tolerance,
 };
 
-impl Approx for HandleIter<'_, Face> {
+impl Approx for &Handles<Face> {
     type Approximation = BTreeSet<FaceApprox>;
     type Cache = EdgeApproxCache;
 
diff --git a/crates/fj-core/src/algorithms/approx/shell.rs b/crates/fj-core/src/algorithms/approx/shell.rs
index 0bbef83f9..1fbb21121 100644
--- a/crates/fj-core/src/algorithms/approx/shell.rs
+++ b/crates/fj-core/src/algorithms/approx/shell.rs
@@ -15,6 +15,6 @@ impl Approx for &Shell {
         tolerance: impl Into<Tolerance>,
         cache: &mut Self::Cache,
     ) -> Self::Approximation {
-        self.faces().iter().approx_with_cache(tolerance, cache)
+        self.faces().approx_with_cache(tolerance, cache)
     }
 }

From 1a38e1b95881070bc29b4ac3ab633733b9def0f1 Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:31:43 +0200
Subject: [PATCH 19/22] Refactor to simplify

---
 crates/fj-core/src/algorithms/intersect/face_point.rs | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/crates/fj-core/src/algorithms/intersect/face_point.rs b/crates/fj-core/src/algorithms/intersect/face_point.rs
index 6d09bb6f7..50fd269f0 100644
--- a/crates/fj-core/src/algorithms/intersect/face_point.rs
+++ b/crates/fj-core/src/algorithms/intersect/face_point.rs
@@ -31,8 +31,7 @@ impl Intersect for (&Face, &Point<2>) {
                 .edges()
                 .iter()
                 .last()
-                .cloned()
-                .and_then(|edge| (&ray, &edge).intersect());
+                .and_then(|edge| (&ray, edge).intersect());
 
             for (edge, next_edge) in cycle.edges().pairs() {
                 let hit = (&ray, edge).intersect();

From a2bd7ab4c3e824668e7635aa91d54a2fcc01ba1e Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:33:08 +0200
Subject: [PATCH 20/22] Move comment

---
 crates/fj-core/src/objects/handles.rs | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/crates/fj-core/src/objects/handles.rs b/crates/fj-core/src/objects/handles.rs
index 63fbd1f13..51769078e 100644
--- a/crates/fj-core/src/objects/handles.rs
+++ b/crates/fj-core/src/objects/handles.rs
@@ -113,6 +113,14 @@ where
 }
 
 impl<'r, T> IntoIterator for &'r Handles<T> {
+    // You might wonder why we're returning references to handles here, when
+    // `Handle` already is kind of reference, and easily cloned.
+    //
+    // Most of the time that doesn't make a difference, but there are use cases
+    // where dealing with owned `Handle`s is inconvenient, for example when
+    // using iterator adapters. You can't return a reference to the argument of
+    // an adapter's closure, if you own that argument. You can, if you just
+    // reference the argument.
     type Item = &'r Handle<T>;
     type IntoIter = HandleIter<'r, T>;
 
@@ -169,14 +177,6 @@ impl<'r, T> HandleIter<'r, T> {
 }
 
 impl<'r, T> Iterator for HandleIter<'r, T> {
-    // You might wonder why we're returning references to handles here, when
-    // `Handle` already is kind of reference, and easily cloned.
-    //
-    // Most of the time that doesn't make a difference, but there are use cases
-    // where dealing with owned `Handle`s is inconvenient, for example when
-    // using iterator adapters. You can't return a reference to the argument of
-    // an adapter's closure, if you own that argument. You can, if you just
-    // reference the argument.
     type Item = &'r Handle<T>;
 
     fn next(&mut self) -> Option<Self::Item> {

From 921b3d9884e68be0338d361ccb3302a82393e70c Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:35:17 +0200
Subject: [PATCH 21/22] Bypass `HandleIter`

---
 crates/fj-core/src/objects/handles.rs | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/crates/fj-core/src/objects/handles.rs b/crates/fj-core/src/objects/handles.rs
index 51769078e..8301243b7 100644
--- a/crates/fj-core/src/objects/handles.rs
+++ b/crates/fj-core/src/objects/handles.rs
@@ -1,4 +1,4 @@
-use std::{collections::BTreeSet, fmt::Debug};
+use std::{collections::BTreeSet, fmt::Debug, slice};
 
 use itertools::Itertools;
 
@@ -90,11 +90,8 @@ impl<T> Handles<T> {
     }
 
     /// Access an iterator over the handles
-    pub fn iter(&self) -> HandleIter<T> {
-        HandleIter {
-            handles: self,
-            next_index: 0,
-        }
+    pub fn iter(&self) -> slice::Iter<Handle<T>> {
+        self.inner.iter()
     }
 
     /// Return iterator over the pairs of all handles
@@ -122,7 +119,7 @@ impl<'r, T> IntoIterator for &'r Handles<T> {
     // an adapter's closure, if you own that argument. You can, if you just
     // reference the argument.
     type Item = &'r Handle<T>;
-    type IntoIter = HandleIter<'r, T>;
+    type IntoIter = slice::Iter<'r, Handle<T>>;
 
     fn into_iter(self) -> Self::IntoIter {
         self.iter()

From 4b0d6cf7fa2e5cf4150afb87f8902eb4f4f41fdf Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Fri, 22 Sep 2023 12:35:41 +0200
Subject: [PATCH 22/22] Remove `HandleIter`

---
 crates/fj-core/src/objects/handles.rs | 75 ---------------------------
 crates/fj-core/src/objects/mod.rs     |  2 +-
 2 files changed, 1 insertion(+), 76 deletions(-)

diff --git a/crates/fj-core/src/objects/handles.rs b/crates/fj-core/src/objects/handles.rs
index 8301243b7..a2ed0c6f1 100644
--- a/crates/fj-core/src/objects/handles.rs
+++ b/crates/fj-core/src/objects/handles.rs
@@ -125,78 +125,3 @@ impl<'r, T> IntoIterator for &'r Handles<T> {
         self.iter()
     }
 }
-
-/// An iterator over handles to objects
-///
-/// This struct is returned by the respective methods of all objects that
-/// reference multiple objects of the same type.
-pub struct HandleIter<'r, T> {
-    handles: &'r Handles<T>,
-    next_index: usize,
-}
-
-impl<'r, T> HandleIter<'r, T> {
-    /// Return the n-th item
-    ///
-    /// This method is unaffected by any previous calls to `next`.
-    pub fn nth(&self, index: usize) -> Option<&Handle<T>> {
-        self.handles.nth(index)
-    }
-
-    /// Return the n-th item, treating the iterator as circular
-    ///
-    /// If the length of the iterator is `i`, then retrieving the i-th edge
-    /// using this method, is the same as retrieving the 0-th one.
-    ///
-    /// This method is unaffected by any previous calls to `next`.
-    pub fn nth_circular(&self, index: usize) -> &Handle<T> {
-        self.handles.nth_circular(index)
-    }
-
-    /// Return the index of the item, if it is in this iterator
-    ///
-    /// This method is unaffected by any previous calls to `next`.
-    pub fn index_of(&self, handle: &Handle<T>) -> Option<usize> {
-        self.handles.index_of(handle)
-    }
-
-    /// Access the item after the provided one
-    ///
-    /// Returns `None`, if the provided item is not in this iterator.
-    pub fn after(&self, handle: &Handle<T>) -> Option<&Handle<T>> {
-        self.handles.after(handle)
-    }
-
-    /// Return iterator over the pairs of the remaining items in this iterator
-    pub fn pairs(self) -> impl Iterator<Item = (&'r Handle<T>, &'r Handle<T>)> {
-        self.handles.pairs()
-    }
-}
-
-impl<'r, T> Iterator for HandleIter<'r, T> {
-    type Item = &'r Handle<T>;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        let handle = self.handles.inner.get(self.next_index);
-        self.next_index += 1;
-        handle
-    }
-
-    fn size_hint(&self) -> (usize, Option<usize>) {
-        let size = self.handles.inner.len();
-        (size, Some(size))
-    }
-}
-
-impl<T> ExactSizeIterator for HandleIter<'_, T> {}
-
-// Deriving won't work, as that only derives `Clone` where `T: Clone`. But
-// `HandleIter` can be `Clone`d unconditionally.
-impl<T> Clone for HandleIter<'_, T> {
-    fn clone(&self) -> Self {
-        Self {
-            handles: self.handles,
-            next_index: self.next_index,
-        }
-    }
-}
diff --git a/crates/fj-core/src/objects/mod.rs b/crates/fj-core/src/objects/mod.rs
index 53c588eda..033d91c0d 100644
--- a/crates/fj-core/src/objects/mod.rs
+++ b/crates/fj-core/src/objects/mod.rs
@@ -46,7 +46,7 @@ mod set;
 mod stores;
 
 pub use self::{
-    handles::{HandleIter, Handles},
+    handles::Handles,
     kinds::{
         curve::Curve,
         cycle::Cycle,