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

LocalTime math can overflow ints too easily #8

Closed
mlms13 opened this issue Aug 11, 2019 · 2 comments
Closed

LocalTime math can overflow ints too easily #8

mlms13 opened this issue Aug 11, 2019 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@mlms13
Copy link
Member

mlms13 commented Aug 11, 2019

Currently, the math helpers for LocalTime (e.g. addHours) convert all of the values into milliseconds, then apply the change consistently in milliseconds. This means that addHours(1000) will probably overflow and produce an incorrect result.

Why do you want to add 1000 hours to a time-of-day? I have no idea, but there's no reason we should give back the wrong answer. We should switch to our int division function that returns both quotient and remainder, so that we can apply all of the full hours first, followed by the minute remainders, etc.

@mlms13 mlms13 added the bug Something isn't working label Aug 11, 2019
@mlms13 mlms13 self-assigned this Aug 11, 2019
@mlms13 mlms13 added this to To do in 0.1.0 (Initial Release) via automation Aug 11, 2019
@mlms13 mlms13 moved this from To do to In progress in 0.1.0 (Initial Release) Aug 12, 2019
@mlms13
Copy link
Member Author

mlms13 commented Aug 12, 2019

Confirmed that this is an issue with a failing test... fix incoming.

@mlms13
Copy link
Member Author

mlms13 commented Aug 12, 2019

Fixed, and fixed an unrelated bug with subtracting values.

@mlms13 mlms13 closed this as completed Aug 12, 2019
0.1.0 (Initial Release) automation moved this from In progress to Done Aug 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Development

No branches or pull requests

1 participant