Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify cycle/edge/curve approximation code #1013

Merged
merged 7 commits into from
Aug 29, 2022
Merged

Simplify cycle/edge/curve approximation code #1013

merged 7 commits into from
Aug 29, 2022

Conversation

hannobraun
Copy link
Owner

Reshuffles responsibilities between cycle, edge, and curve approximation. Aligns the approximation code for the different curve types, greatly simplifying edge approximation.

The resulting code is a bit more elegant, leading to a reduction in lines of code and complexity. It also paves the way for simplifying edge representation by removing the "continuous edge" edge case (no pun intended).

Since there is always going to be a next edge in the cycle, whose first
point is the previous edge's last point, there is no need to include the
last point in the approximation in the first place.
The function existed only for the test, but now I'm not sure how to
uphold its properties that made it testable going forward. I think just
removing the test is an okay solution for now, since there are
higher-level tests covering the same code.
This serves no purpose, except making further changes I have in mind
easier.
With this commit, all curve approximations work the same now, making
edge approximation much simpler.
@hannobraun hannobraun enabled auto-merge August 29, 2022 18:44
@hannobraun hannobraun merged commit 801af9c into main Aug 29, 2022
@hannobraun hannobraun deleted the approx5 branch August 29, 2022 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant