reazen / relude-eon

A datetime library for the ages in ReasonML
MIT License
12 stars 4 forks source link

LocalTime math can overflow ints too easily #8

Closed mlms13 closed 5 years ago

mlms13 commented 5 years ago

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 commented 5 years ago

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

mlms13 commented 5 years ago

Fixed, and fixed an unrelated bug with subtracting values.