From a5fb7beb8088adf56eb5c5216b2951b247cfa892 Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Thu, 10 Mar 2022 18:49:11 +0100
Subject: [PATCH] Return `ValidationError` from `Cycles::add`

---
 src/kernel/shape/cycles.rs | 78 ++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 37 deletions(-)

diff --git a/src/kernel/shape/cycles.rs b/src/kernel/shape/cycles.rs
index 9ae2f12e9..ef11af505 100644
--- a/src/kernel/shape/cycles.rs
+++ b/src/kernel/shape/cycles.rs
@@ -2,7 +2,7 @@ use crate::kernel::topology::edges::Cycle;
 
 use super::{
     handle::{Handle, Storage},
-    CyclesInner, EdgesInner, ValidationResult,
+    CyclesInner, EdgesInner, ValidationError, ValidationResult,
 };
 
 /// The cycles of a shape
@@ -26,10 +26,9 @@ impl Cycles<'_> {
     /// - That there exists no duplicate cycle, with the same edges.
     pub fn add(&mut self, cycle: Cycle) -> ValidationResult<Cycle> {
         for edge in &cycle.edges {
-            assert!(
-                self.edges.contains(edge.storage()),
-                "Cycle validation failed: {edge:?} is not part of the shape",
-            );
+            if !self.edges.contains(edge.storage()) {
+                return Err(ValidationError::Structural);
+            }
         }
 
         let storage = Storage::new(cycle);
@@ -49,49 +48,54 @@ impl Cycles<'_> {
 mod tests {
     use crate::{
         kernel::{
-            shape::Shape,
-            topology::{edges::Cycle, vertices::Vertex},
+            shape::{handle::Handle, Shape, ValidationError},
+            topology::{
+                edges::{Cycle, Edge},
+                vertices::Vertex,
+            },
         },
         math::Point,
     };
 
     #[test]
     fn add() -> anyhow::Result<()> {
-        let mut shape = Shape::new();
+        struct TestShape {
+            inner: Shape,
+            edge: Handle<Edge>,
+        }
 
-        let a = shape.geometry().add_point(Point::from([0., 0., 0.]))?;
-        let b = shape.geometry().add_point(Point::from([1., 0., 0.]))?;
+        impl TestShape {
+            fn new() -> anyhow::Result<Self> {
+                let mut inner = Shape::new();
 
-        let a = shape.vertices().add(Vertex { point: a })?;
-        let b = shape.vertices().add(Vertex { point: b })?;
+                let a =
+                    inner.geometry().add_point(Point::from([0., 0., 0.]))?;
+                let b =
+                    inner.geometry().add_point(Point::from([1., 0., 0.]))?;
 
-        let edge = shape.edges().add_line_segment([a, b])?;
+                let a = inner.vertices().add(Vertex { point: a })?;
+                let b = inner.vertices().add(Vertex { point: b })?;
 
-        shape.cycles().add(Cycle { edges: vec![edge] })?;
+                let edge = inner.edges().add_line_segment([a, b])?;
 
-        Ok(())
-    }
+                Ok(Self { inner, edge })
+            }
+        }
 
-    #[test]
-    #[should_panic]
-    fn add_invalid() {
-        let mut shape = Shape::new();
-        let mut other = Shape::new();
-
-        let a = shape
-            .geometry()
-            .add_point(Point::from([0., 0., 0.]))
-            .unwrap();
-        let b = shape
-            .geometry()
-            .add_point(Point::from([1., 0., 0.]))
-            .unwrap();
-
-        let a = other.vertices().add(Vertex { point: a }).unwrap();
-        let b = other.vertices().add(Vertex { point: b }).unwrap();
-
-        let edge = other.edges().add_line_segment([a, b]).unwrap();
-
-        shape.cycles().add(Cycle { edges: vec![edge] }).unwrap();
+        let mut shape = TestShape::new()?;
+        let other = TestShape::new()?;
+
+        // Trying to refer to edge that is not from the same shape. Should fail.
+        let result = shape.inner.cycles().add(Cycle {
+            edges: vec![other.edge],
+        });
+        assert!(matches!(result, Err(ValidationError::Structural)));
+
+        // Referring to edge that *is* from the same shape. Should work.
+        shape.inner.cycles().add(Cycle {
+            edges: vec![shape.edge],
+        })?;
+
+        Ok(())
     }
 }