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

Handle date difference for ends of months #553

Merged
merged 1 commit into from Jun 17, 2019

Conversation

Bigpet
Copy link
Contributor

@Bigpet Bigpet commented Jun 16, 2019

Fixes #552

Description of the changes:

The incrementally calling Window::Globalization::Calendar::AddMonths
resulted in a negative value for GetDifferenceInDays which was then
assigned to an unsigned variable daysDiff.

One example of the issue when running the calculator in UTC+2 was the
difference between July 31st and December 30th.

The initial guess was 4 months which then landed on November 30th.
This date was stored and then in the loop incremeted by one month.
This then landed precisely on the end date December 30th.
After the loop the final value is then used July 31st + 5 months
which results in the 31st of December.
The resulting difference of -1 days is then assigned to the unsigned
value daysDiff.

This commit makes the minimal changes to remedy this bug.
It makes sure to only ever call AddMonths with the same starting date
instead of incrementally to different dates.

This commit makes the minimal changes to remedy this bug.
It makes sure to only ever call AddMonths with the same starting date
instead of incrementally to different dates.

How changes were validated:

  • manually tested dates near the end of month where this incremental calculation would be triggered

The incrementally calling `Window::Globalization::Calendar::AddMonths`
resulted in a negative value for `GetDifferenceInDays` which was then
assigned to an unsigned variable `daysDiff`.

One example of the issue when running the calculator in UTC+2 was the
difference between July 31st and December 30th.

The initial guess was 4 months which then landed on November 30th.
This date was stored and then in the loop incremeted by one month.
This then landed precisely on the end date December 30th.
After the loop the final value is then used July 31st + 5 months
which results in the 31st of December.
The resulting difference of -1 days is then assigned to the unsigned
value `daysDiff`.

This commit makes the minimal changes to remedy this bug.
It makes sure to only ever call `AddMonths` with the same starting date
instead of incrementally to different dates.

fixes microsoft#552
Copy link
Contributor

@EriWong EriWong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this!

@EriWong EriWong merged commit dab589b into microsoft:master Jun 17, 2019
@wilgert
Copy link

wilgert commented Jun 19, 2019

Maybe also add a Unit Test that replicates the bug? So it will never appear again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issue is a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Date difference calculation wrong in certain cases
4 participants