Skip to content

Commit

Permalink
Remove HalfEdge::from_curve_and_vertices
Browse files Browse the repository at this point in the history
This constructor is a bit problematic, for multiple reasons:

- It fulfills a role that is better fitted for the builder API.
- It takes away control over the `GlobalVertex` construction from the
  caller.

That second point is a problem, due to the work that needs to happen to
address #1079. This commit doesn't solve that, but the builder API
provides a better framework to do so than this constructor did.
  • Loading branch information
hannobraun committed Sep 21, 2022
1 parent 6c991c8 commit adf1017
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 23 deletions.
8 changes: 5 additions & 3 deletions crates/fj-kernel/src/algorithms/validate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ mod tests {
use crate::{
algorithms::validate::{Validate, ValidationConfig, ValidationError},
objects::{
Curve, GlobalCurve, GlobalVertex, HalfEdge, Surface, SurfaceVertex,
Vertex,
Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface,
SurfaceVertex, Vertex,
},
path::{GlobalPath, SurfacePath},
stores::Stores,
Expand Down Expand Up @@ -236,7 +236,9 @@ mod tests {
);
let vertices = [a, b];

let half_edge = HalfEdge::from_curve_and_vertices(curve, vertices);
let global_edge = GlobalEdge::builder()
.build_from_curve_and_vertices(&curve, &vertices);
let half_edge = HalfEdge::new(curve, vertices, global_edge);

let result =
half_edge.clone().validate_with_config(&ValidationConfig {
Expand Down
10 changes: 8 additions & 2 deletions crates/fj-kernel/src/builder/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ impl<'a> HalfEdgeBuilder<'a> {
)
};

HalfEdge::from_curve_and_vertices(curve, vertices)
let global_form = GlobalEdge::builder()
.build_from_curve_and_vertices(&curve, &vertices);

HalfEdge::new(curve, vertices, global_form)
}

/// Build a line segment from two points
Expand Down Expand Up @@ -121,7 +124,10 @@ impl<'a> HalfEdgeBuilder<'a> {
]
};

HalfEdge::from_curve_and_vertices(curve, vertices)
let global_form = GlobalEdge::builder()
.build_from_curve_and_vertices(&curve, &vertices);

HalfEdge::new(curve, vertices, global_form)
}
}

Expand Down
18 changes: 0 additions & 18 deletions crates/fj-kernel/src/objects/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ impl HalfEdge {

/// Create a new instance of `HalfEdge`
///
/// If you only have a curve and the edge vertices, please check out
/// [`HalfEdge::from_curve_and_vertices`], which is a convenience wrapper
/// around this method, which creates an instance of [`GlobalEdge`].
///
/// # Panics
///
/// Panics, if the provided `vertices` are not defined on the same curve as
Expand Down Expand Up @@ -76,20 +72,6 @@ impl HalfEdge {
}
}

/// Create a new instance of `HalfEdge` from a curve and vertices
///
/// The [`GlobalEdge`] instance is created from the provided curve and
/// vertices. Please refer to [`HalfEdge::new`], if you already have a
/// [`GlobalEdge`] instance that you can provide.
pub fn from_curve_and_vertices(
curve: Curve,
vertices: [Vertex; 2],
) -> Self {
let global = GlobalEdge::builder()
.build_from_curve_and_vertices(&curve, &vertices);
Self::new(curve, vertices, global)
}

/// Access the curve that defines the half-edge's geometry
///
/// The edge can be a segment of the curve that is bounded by two vertices,
Expand Down

0 comments on commit adf1017

Please sign in to comment.