From ae83a69f645236b671a4fe6e07c6cb2643a195e8 Mon Sep 17 00:00:00 2001 From: Iskren Chernev Date: Mon, 29 Dec 2014 13:18:18 +0200 Subject: [PATCH] Reimplement month diff logic The new algorithm to compute month differences between two moments (used in `diff(a, 'month|quarter|year')`), is as follows: - take the whole number of months between the two dates (ignoring day-of-month and hour) -- whole-month-diff - the above is adjusted by how much the second date is away from the whole-month-diff-date (the date where the result would be exactly whole-month-diff) This algorithm makes sure that dates that are exact number of months apart would return whole numbers (like previous algorithm), but also fixes a few edge cases that the old agorithm failed to do, like when the first moment is close to the end of a long month and the second moment was in the begining of a short month (end of Jan to begining of Feb for example). Fixes #2026 --- moment.js | 37 +++++++++++++--------- test/moment/diff.js | 76 ++++++++++++++++++++++++++++----------------- 2 files changed, 71 insertions(+), 42 deletions(-) diff --git a/moment.js b/moment.js index e4fbb6e87..7033f78a6 100644 --- a/moment.js +++ b/moment.js @@ -356,6 +356,26 @@ }; } + function monthDiff(a, b) { + // difference in months + var wholeMonthDiff = ((b.year() - a.year()) * 12) + (b.month() - a.month()), + // b is in (anchor - 1 month, anchor + 1 month) + anchor = a.clone().add(wholeMonthDiff, 'months'), + anchor2, adjust; + + if (b - anchor < 0) { + anchor2 = a.clone().add(wholeMonthDiff - 1, 'months'); + // linear across the month + adjust = (b - anchor) / (anchor - anchor2); + } else { + anchor2 = a.clone().add(wholeMonthDiff + 1, 'months'); + // linear across the month + adjust = (b - anchor) / (anchor2 - anchor); + } + + return -(wholeMonthDiff + adjust); + } + while (ordinalizeTokens.length) { i = ordinalizeTokens.pop(); formatTokenFunctions[i + 'o'] = ordinalizeToken(formatTokenFunctions[i], i); @@ -2268,30 +2288,19 @@ diff : function (input, units, asFloat) { var that = makeAs(input, this), zoneDiff = (that.utcOffset() - this.utcOffset()) * 6e4, - diff, output, daysAdjust; + anchor, diff, output, daysAdjust; units = normalizeUnits(units); if (units === 'year' || units === 'month' || units === 'quarter') { - // average number of days in the months in the given dates - diff = (this.daysInMonth() + that.daysInMonth()) * 432e5; // 24 * 60 * 60 * 1000 / 2 - // difference in months - output = ((this.year() - that.year()) * 12) + (this.month() - that.month()); - // adjust by taking difference in days, average number of days - // and dst in the given months. - daysAdjust = (this - moment(this).startOf('month')) - - (that - moment(that).startOf('month')); - // same as above but with zones, to negate all dst - daysAdjust += ((this.utcOffset() - moment(this).startOf('month').utcOffset()) - - (that.utcOffset() - moment(that).startOf('month').utcOffset())) * 6e4; - output += daysAdjust / diff; + output = monthDiff(this, that); if (units === 'quarter') { output = output / 3; } else if (units === 'year') { output = output / 12; } } else { - diff = (this - that); + diff = this - that; output = units === 'second' ? diff / 1e3 : // 1000 units === 'minute' ? diff / 6e4 : // 1000 * 60 units === 'hour' ? diff / 36e5 : // 1000 * 60 * 60 diff --git a/test/moment/diff.js b/test/moment/diff.js index 5aca304c8..95769585c 100644 --- a/test/moment/diff.js +++ b/test/moment/diff.js @@ -132,33 +132,54 @@ exports.diff = { return; } - test.expect(16); - a = dst.moment; b = a.clone().utc().add(12, 'hours').local(); daysInMonth = (a.daysInMonth() + b.daysInMonth()) / 2; - equal(test, b.diff(a, 'ms', true), 12 * 60 * 60 * 1000, 'ms diff across DST'); - equal(test, b.diff(a, 's', true), 12 * 60 * 60, 'second diff across DST'); - equal(test, b.diff(a, 'm', true), 12 * 60, 'minute diff across DST'); - equal(test, b.diff(a, 'h', true), 12, 'hour diff across DST'); - equal(test, b.diff(a, 'd', true), (12 - dst.diff) / 24, 'day diff across DST'); - equal(test, b.diff(a, 'w', true), (12 - dst.diff) / 24 / 7, 'week diff across DST'); - equal(test, b.diff(a, 'M', true), (12 - dst.diff) / 24 / daysInMonth, 'month diff across DST'); - equal(test, b.diff(a, 'y', true), (12 - dst.diff) / 24 / daysInMonth / 12, 'year diff across DST'); - + test.equal(b.diff(a, 'milliseconds', true), 12 * 60 * 60 * 1000, + 'ms diff across DST'); + test.equal(b.diff(a, 'seconds', true), 12 * 60 * 60, + 'second diff across DST'); + test.equal(b.diff(a, 'minutes', true), 12 * 60, + 'minute diff across DST'); + test.equal(b.diff(a, 'hours', true), 12, + 'hour diff across DST'); + test.equal(b.diff(a, 'days', true), (12 - dst.diff) / 24, + 'day diff across DST'); + equal(test, b.diff(a, 'weeks', true), (12 - dst.diff) / 24 / 7, + 'week diff across DST'); + test.ok(0.95 / (2 * 31) < b.diff(a, 'months', true), + 'month diff across DST, lower bound'); + test.ok(b.diff(a, 'month', true) < 1.05 / (2 * 28), + 'month diff across DST, upper bound'); + test.ok(0.95 / (2 * 31 * 12) < b.diff(a, 'years', true), + 'year diff across DST, lower bound'); + test.ok(b.diff(a, 'year', true) < 1.05 / (2 * 28 * 12), + 'year diff across DST, upper bound'); a = dst.moment; b = a.clone().utc().add(12 + dst.diff, 'hours').local(); daysInMonth = (a.daysInMonth() + b.daysInMonth()) / 2; - equal(test, b.diff(a, 'ms', true), (12 + dst.diff) * 60 * 60 * 1000, 'ms diff across DST'); - equal(test, b.diff(a, 's', true), (12 + dst.diff) * 60 * 60, 'second diff across DST'); - equal(test, b.diff(a, 'm', true), (12 + dst.diff) * 60, 'minute diff across DST'); - equal(test, b.diff(a, 'h', true), (12 + dst.diff), 'hour diff across DST'); - equal(test, b.diff(a, 'd', true), 12 / 24, 'day diff across DST'); - equal(test, b.diff(a, 'w', true), 12 / 24 / 7, 'week diff across DST'); - equal(test, b.diff(a, 'M', true), 12 / 24 / daysInMonth, 'month diff across DST'); - equal(test, b.diff(a, 'y', true), 12 / 24 / daysInMonth / 12, 'year diff across DST'); + test.equal(b.diff(a, 'milliseconds', true), + (12 + dst.diff) * 60 * 60 * 1000, + 'ms diff across DST'); + test.equal(b.diff(a, 'seconds', true), (12 + dst.diff) * 60 * 60, + 'second diff across DST'); + test.equal(b.diff(a, 'minutes', true), (12 + dst.diff) * 60, + 'minute diff across DST'); + test.equal(b.diff(a, 'hours', true), (12 + dst.diff), + 'hour diff across DST'); + test.equal(b.diff(a, 'days', true), 12 / 24, 'day diff across DST'); + equal(test, b.diff(a, 'weeks', true), 12 / 24 / 7, + 'week diff across DST'); + test.ok(0.95 / (2 * 31) < b.diff(a, 'months', true), + 'month diff across DST, lower bound'); + test.ok(b.diff(a, 'month', true) < 1.05 / (2 * 28), + 'month diff across DST, upper bound'); + test.ok(0.95 / (2 * 31 * 12) < b.diff(a, 'years', true), + 'year diff across DST, lower bound'); + test.ok(b.diff(a, 'year', true) < 1.05 / (2 * 28 * 12), + 'year diff across DST, upper bound'); test.done(); }, @@ -211,17 +232,16 @@ exports.diff = { }, 'month diffs' : function (test) { - test.expect(8); - // due to floating point math errors, these tests just need to be accurate within 0.00000001 - equal(test, moment([2012, 0, 1]).diff([2012, 1, 1], 'months', true), -1, 'Jan 1 to Feb 1 should be 1 month'); + test.equal(moment([2012, 0, 1]).diff([2012, 1, 1], 'months', true), -1, 'Jan 1 to Feb 1 should be 1 month'); equal(test, moment([2012, 0, 1]).diff([2012, 0, 1, 12], 'months', true), -0.5 / 31, 'Jan 1 to Jan 1 noon should be 0.5 / 31 months'); - equal(test, moment([2012, 0, 15]).diff([2012, 1, 15], 'months', true), -1, 'Jan 15 to Feb 15 should be 1 month'); - equal(test, moment([2012, 0, 28]).diff([2012, 1, 28], 'months', true), -1, 'Jan 28 to Feb 28 should be 1 month'); - equal(test, moment([2012, 0, 31]).diff([2012, 1, 29], 'months', true), -1 + (2 / 30), 'Jan 31 to Feb 29 should be 1 - (2 / 30) months'); - equal(test, moment([2012, 0, 31]).diff([2012, 2, 1], 'months', true), -2 + (30 / 31), 'Jan 31 to Mar 1 should be 2 - (30 / 31) months'); - equal(test, moment([2012, 0, 31]).diff([2012, 2, 1, 12], 'months', true), -2 + (29.5 / 31), 'Jan 31 to Mar 1 should be 2 - (29.5 / 31) months'); + test.equal(moment([2012, 0, 15]).diff([2012, 1, 15], 'months', true), -1, 'Jan 15 to Feb 15 should be 1 month'); + test.equal(moment([2012, 0, 28]).diff([2012, 1, 28], 'months', true), -1, 'Jan 28 to Feb 28 should be 1 month'); + test.ok(moment([2012, 0, 31]).diff([2012, 1, 29], 'months', true), -1, 'Jan 31 to Feb 29 should be 1 month'); + test.ok(-1 > moment([2012, 0, 31]).diff([2012, 2, 1], 'months', true), 'Jan 31 to Mar 1 should be more than 1 month'); + test.ok(-30 / 28 < moment([2012, 0, 31]).diff([2012, 2, 1], 'months', true), 'Jan 31 to Mar 1 should be less than 1 month and 1 day'); equal(test, moment([2012, 0, 1]).diff([2012, 0, 31], 'months', true), -(30 / 31), 'Jan 1 to Jan 31 should be 30 / 31 months'); + test.ok(0 < moment('2014-02-01').diff(moment('2014-01-31'), 'months', true), 'jan-31 to feb-1 diff is positive'); test.done(); }, @@ -253,7 +273,7 @@ exports.diff = { equal(test, moment([2012, 0, 31]).diff([2013, 6, 31], 'years', true), -1.5, 'Jan 31 2012 to Jul 31 2013 should be 1.5 years'); equal(test, moment([2012, 0, 1]).diff([2013, 0, 1, 12], 'years', true), -1 - (0.5 / 31) / 12, 'Jan 1 2012 to Jan 1 2013 noon should be 1+(0.5 / 31) / 12 years'); equal(test, moment([2012, 0, 1]).diff([2013, 6, 1, 12], 'years', true), -1.5 - (0.5 / 31) / 12, 'Jan 1 2012 to Jul 1 2013 noon should be 1.5+(0.5 / 31) / 12 years'); - equal(test, moment([2012, 1, 29]).diff([2013, 1, 28], 'years', true), -1 + (1 / 28.5) / 12, 'Feb 29 2012 to Feb 28 2013 should be 1-(1 / 28.5) / 12 years'); + equal(test, moment([2012, 1, 29]).diff([2013, 1, 28], 'years', true), -1, 'Feb 29 2012 to Feb 28 2013 should be 1-(1 / 28.5) / 12 years'); test.done(); } -- 2.47.2