Fix month and year diffs #571

Merged
merged 3 commits into from Jan 16, 2013

Projects

None yet

3 participants

@timrwood
Member

This is a potential solution to the buggy month/year diffs. However, when working on this I ran into some tricky problems, so I want to outline them here for posterity.

There are a few different ways to calculate the difference in years. The first way is to compare the percentages through the year.

startPercent = 45 / 365; // 0.1232876712328767
endPercent = 57 / 365; // 0.15616438356164383
diffPercent = endPercent - startPercent; // 0.03287671232876713

However, this has unexpected results when crossing leap years due to the extra day. 45 days into the year is a slightly smaller percentage on years with 366 days than years with 365 days.

startPercent = 45 / 365; // 0.1232876712328767
endPercent = 45 / 366; // 0.12295081967213115
diffPercent = endPercent - startPercent; // -0.0003368515607455602

The solution I am proposing is that we use the percentages through the months divided by 12.

We calculate the percentage through the year with month accuracy (0/12, 1/12, 2/12 ... 11/12).

We then calculate the percentages through the start and end months with millisecond accuracy. For example, (189798626 / (31 * 24 * 60 * 60 * 1000)).

Then, the combination of those two diffs results in a slightly more expected result.

An added bonus to this methodology is that we can use the same calculation for difference in months. Month diff = year diff * 12.

@timrwood
Member

One downfall of this method is that the same issue of having slightly different percentages still exists between months with different lengths, so July 15 to August 15 != 1 month.

This also includes diffs between February and February when one is on a leap year and one is not. Feb 1 2012 to Feb 1 2013 != 1.

@ichernev
Contributor

Nice hack :) I just want to lay down my thoughts on this, I have to try and implement them to see if it is worth it.
In order not to have the strange side effects you outline, we may use a duration in months + days/days in month (float). For example 5th Jan to 6th Feb is 1 month + 1/28 days (or 29, because we're counting Feb's days) . But 5th Jan to 4th Feb is 0 months + 30/31 days (because we're counting Jan's days).

We can use the same logic for years (keep full years + days/[days in this/previous year]). It requires a check to see if the date of month/year is before the start day/date -- in the 5th Jan to 4th Feb case, we check that 4 < 5, so we count the month before Feb's days, if >= 5, then we count Febs days.

I'm not sure that trading one set of issues (the ones we have now) with the ones created by the current solution is worth while. I'm also not sure if the more complicated solution outlined above is worth implementing to fix some minor glitches like the ones we face now.

EDIT: Argh! My solution won't work well on diffs near the end of the month, like 31Jan to 28Feb would be 28/31 of a month, and 31Jan to 1Mar would be 1 month + 30/28 :)

@timrwood
Member

Hmm, I like your idea, what about something like this instead.

I think the most common use case for year diffs is birthday calculation and age gating, thus I think we should optimize for comparing two moments with the same day of the month.

So Jan 15 to Feb 15 should be exactly 1 month. Feb 28 to Mar 28 should be exactly 1 month. Feb 28 2011 to Feb 28 2012 should be exactly 1 year.

There are two numbers used to calculate months/years. The integer difference and the partial difference.

The integer difference would stay the same as in the pull request. this.month() - that.month().

The partial difference is changed to the following.

We do not use % through month A minus % through month B.

We calculate the difference between the number of days through month A with the number of days through month B and then divide that by the average of the month lengths. It's hard to describe, so here are some examples.

// Jan 1 - Jan 3
1 - 3 = -2;            // difference in days
(31 + 31) / 2 = 31;    // average month length
-2 / 31 = -0.0654;     // the partial difference in months
0 - 0 = 0;             // the integer difference in months
0 + -0.0654 = -0.0654; // the total difference in months
// Jan 15 - Feb 15
15 - 15 = 0;          // difference in days
(31 + 28) / 2 = 29.5; // average month length
0 / 29.5 = 0;         // the partial difference in months
0 - 1 = -1;           // the integer difference in months
-1 + 0 = -1;          // the total difference in months
// Jan 31 - Feb 28
31 - 28 = 3;          // difference in days
(31 + 28) / 2 = 29.5; // average month length
3 / 29.5 = 0.101;     // the partial difference in months
0 - 1 = -1;           // the integer difference in months
-1 + 0.101 = -0.899;  // the total difference in months
// Jan 31 - Mar 1
31 - 1 = 30;           // difference in days
(31 + 31) / 2 = 31;    // average month length
30 / 31 = 0.9677;      // the partial difference in months
0 - 2 = -2;            // the integer difference in months
-2 + 0.9677 = -1.0323; // the total difference in months

It does get a little weird toward the ends of two months with different lengths, but I think there is always going to be some weirdness in assigning a numerical value to something like this.

Thoughts?

@timrwood timrwood referenced this pull request Jan 15, 2013
Closed

Feature/diff duration #345

@ichernev
Contributor

Yes, your proposal does work for full months, and it does provide some results in the other cases. I thought about this some more and I come to the conclusion that there are so many contradictory edge cases in the way humans work with time that we'd better not try to work "humanly" for all of them.

So this last proposal has the least surprise, I'm for it.

@timrwood timrwood merged commit dc8d532 into develop Jan 16, 2013

1 check passed

default The Travis build passed
Details
@timrwood timrwood deleted the feature/fix-diff branch Jan 16, 2013
@millermedeiros

the average doesn't really solve the problem, these results are wrong:

// -0.423728813559322
moment('2013-01-23').diff('2013-02-06', 'month', true)
// -0.03531073446327684
moment('2013-01-23').diff('2013-02-06', 'year', true)
  1. Between Jan 23 - Feb 23 there are 31 days (full month), since 14 days elapsed until Feb 06 we should have 14 / 31 = 0.45161290322580644.
  2. Between Jan 23 - Feb 06 there are 14 days, since 2013 isn't a leap year it contains 365 days, 14 / 365 = 0.038356164383561646.

The "fractional year" gets trickier when the diff spans through multiple years and between these years there are leap years. I ended up doing an approximation based on the amount of months in the year, so instead of 14 / 365 I do 14 / 31 / 12 = 0.03763440860215054 (but I might improve this behavior later).

For an implementation reference see mout/mout#82

@millermedeiros

current implementation on mout/mout#82 is accurate for partial months and years. I calculate how many days elapsed on the "partial year" and divide that based on the totalDaysInYear (365 regular years and 366 on leap year)

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