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

Add `Vector2D::try_normalize` and `Vector3D::try_normalize`. #431

Merged
merged 1 commit into from Apr 26, 2020

Conversation

@kyren
Copy link
Contributor

kyren commented Apr 25, 2020

These methods check that the length of the vector is not zero and return None in
that case. It is slightly cleaner than either checking that the vector is
not equal to zero or checking that the length is not zero before calling the
regular normalize method.

If the vector has very small component values, length may be calculated as zero,
and thus the length of the normalized vector will be infinity.

Another possibility here is to copy nalgebra and have a min_length parameter, rather than assuming that minimum length is always zero.

These methods check that the length of the vector is not zero and return None in
that case.  It is slightly cleaner than either checking that the vector is
not *equal* to zero or checking that the length is not zero before calling the
regular `normalize` method.

If the vector has very small component values, length may be calculated as zero,
and thus the length of the normalized vector will be infinity.
@nical
Copy link
Collaborator

nical commented Apr 26, 2020

Looks good to me, thanks!

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Apr 26, 2020

📌 Commit b60359a has been approved by nical

@bors-servo
Copy link
Contributor

bors-servo commented Apr 26, 2020

Testing commit b60359a with merge b37c504...

@bors-servo
Copy link
Contributor

bors-servo commented Apr 26, 2020

☀️ Test successful - checks-travis
Approved by: nical
Pushing b37c504 to master...

@bors-servo bors-servo merged commit b37c504 into servo:master Apr 26, 2020
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.