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

Minor cleanup for radial sorting #493

Merged
merged 2 commits into from
Mar 23, 2023
Merged

Conversation

peterstace
Copy link
Owner

Description

  • The radial sort 'less' function is extracted out into its own standalone function, making the parent function (fixVertex) not need to know about the details of the sort.

  • A new XY method lengthSq is added, that computes the square of the length of the distance between the origin and the XY value.

  • The Cross and lengthSq methods are used for radial sorting rather than reimplementing the logic for those computations.

Check List

Have you:

  • Added unit tests? N/A, relies on existing.

  • Add cmprefimpl tests? (if appropriate?) N/A.

  • Updated release notes? (if appropriate?) Yes.

Related Issue

Benchmark Results

Not run yet.

- The radial sort 'less' function is extracted out into its own
  standalone function, making the parent function (`fixVertex`) not need
  to know about the details of the sort.

- A new `XY` method `lengthSq` is added, that computes the square of the
  length of the distance between the origin and the XY value.

- The `Cross` and `lengthSq` methods are used for radial sorting rather
  than reimplementing the logic for those computations.
@peterstace peterstace self-assigned this Feb 17, 2023
geom/xy.go Outdated Show resolved Hide resolved
Co-authored-by: Albert <26584478+albertteoh@users.noreply.github.com>
@peterstace
Copy link
Owner Author

Thanks for reviewing 😁

@peterstace peterstace merged commit 0380dce into master Mar 23, 2023
@peterstace peterstace deleted the minor_cleanup_for_radial_sorting branch March 23, 2023 23:27
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.

None yet

2 participants