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

Using Math.trunc instead of Math.floor suggests that the argument could be negative. #126

Closed
samreid opened this issue Mar 26, 2024 · 1 comment
Assignees

Comments

@samreid
Copy link
Member

samreid commented Mar 26, 2024

During code review #103, we observed 4 occurrences of Math.trunc, which MDN describes like so:

The Math.trunc() static method returns the integer part of a number by removing any fractional digits.
[...]
Unlike the other three Math methods: Math.floor(), Math.ceil() and Math.round(), the way Math.trunc() works is very simple. It truncates (cuts off) the dot and the digits to the right of it, no matter whether the argument is a positive or negative number.

In the cases we reviewed, it seems like the argument should be non-negative, and we recommend using Math.floor so readers don't misconstrue the argument as possibly negative.

@pixelzoom
Copy link
Contributor

Converted to Math.floor after fuzz testing with assert && assert( Math.trunc( value ) === Math.floor( value ) ) at usage sites.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants