Use ratios to convert bubble days to months and years

diff --git a/src/lib/duration/as.js b/src/lib/duration/as.js
index 258c501e5ab7bacbc873858b7254aa1c58aae819..03ecd6dab2571180214facd47e16097eaa037673 100644 (file)
--- a/src/lib/duration/as.js
+++ b/src/lib/duration/as.js
@@ -1,4 +1,4 @@
-import { daysToYears, yearsToDays } from './bubble';
+import { daysToMonths, monthsToDays } from './bubble';
 import { normalizeUnits } from '../units/aliases';
 import toInt from '../utils/to-int';
 
@@ -11,11 +11,11 @@ export function as (units) {
 
     if (units === 'month' || units === 'year') {
         days   = this._days   + milliseconds / 864e5;
-        months = this._months + daysToYears(days) * 12;
+        months = this._months + daysToMonths(days);
         return units === 'month' ? months : months / 12;
     } else {
         // handle milliseconds separately because of floating point math errors (issue #1867)
-        days = this._days + Math.round(yearsToDays(this._months / 12));
+        days = this._days + Math.round(monthsToDays(this._months));
         switch (units) {
             case 'week'   : return days / 7     + milliseconds / 6048e5;
             case 'day'    : return days         + milliseconds / 864e5;

diff --git a/src/lib/duration/bubble.js b/src/lib/duration/bubble.js
index a71439e2280652f925434e2e6c4d9efcdd820b16..0c4a336ec47d3b89fa3e077be878a4caa7d7c7a4 100644 (file)
--- a/src/lib/duration/bubble.js
+++ b/src/lib/duration/bubble.js
@@ -1,4 +1,5 @@
 import absFloor from '../utils/abs-floor';
+import absCeil from '../utils/abs-ceil';
 import { createUTCDate } from '../create/date-from-array';
 
 export function bubble () {
@@ -6,13 +7,13 @@ export function bubble () {
     var days         = this._days;
     var months       = this._months;
     var data         = this._data;
-    var seconds, minutes, hours, years = 0, monthDaySplit, sign;
+    var seconds, minutes, hours, years, monthsFromDays;
 
     // if we have a mix of positive and negative values, bubble down first
     // check: https://github.com/moment/moment/issues/2166
     if (!((milliseconds >= 0 && days >= 0 && months >= 0) ||
             (milliseconds <= 0 && days <= 0 && months <= 0))) {
-        milliseconds += absFloor(yearsToDays(months / 12) + days) * 864e5;
+        milliseconds += absCeil(monthsToDays(months) + days) * 864e5;
         days = 0;
         months = 0;
     }
@@ -32,20 +33,13 @@ export function bubble () {
 
     days += absFloor(hours / 24);
 
-    // Accurately convert days to years, assume start from year 0.
-    years = absFloor(daysToYears(days));
-    days -= absFloor(yearsToDays(years));
-
-    // Use Jan 1st 1970 as anchor.
-    if (days !== 0) {
-        monthDaySplit = createUTCDate(1970, 0, Math.abs(days) + 1);
-        sign = days < 0 ? -1 : 1;
-        months += sign * monthDaySplit.getUTCMonth();
-        days = sign * (monthDaySplit.getUTCDate() - 1);
-    }
+    // convert days to months
+    monthsFromDays = absFloor(daysToMonths(days));
+    months += monthsFromDays;
+    days -= absCeil(monthsToDays(monthsFromDays));
 
     // 12 months -> 1 year
-    years  += absFloor(months / 12);
+    years = absFloor(months / 12);
     months %= 12;
 
     data.days   = days;
@@ -55,13 +49,13 @@ export function bubble () {
     return this;
 }
 
-export function daysToYears (days) {
+export function daysToMonths (days) {
     // 400 years have 146097 days (taking into account leap year rules)
-    return days * 400 / 146097;
+    // 400 years have 12 months === 4800
+    return days * 4800 / 146097;
 }
 
-export function yearsToDays (years) {
-    // years * 365 + absFloor(years / 4) -
-    //     absFloor(years / 100) + absFloor(years / 400);
-    return years * 146097 / 400;
+export function monthsToDays (months) {
+    // the reverse of daysToMonths
+    return months * 146097 / 4800;
 }

diff --git a/src/lib/utils/abs-ceil.js b/src/lib/utils/abs-ceil.js
new file mode 100644 (file)
index 0000000..7cf9329
--- /dev/null
+++ b/src/lib/utils/abs-ceil.js
@@ -0,0 +1,7 @@
+export default function absCeil (number) {
+    if (number < 0) {
+        return Math.floor(number);
+    } else {
+        return Math.ceil(number);
+    }
+}

diff --git a/src/test/moment/duration.js b/src/test/moment/duration.js
index 6e8df5fc10768a97a3450fcffa81007c364d8385..bb9d066ac56440b732a9dc4f94632735375950d9 100644 (file)
--- a/src/test/moment/duration.js
+++ b/src/test/moment/duration.js
@@ -220,7 +220,7 @@ test('instatiation from serialized C# TimeSpan maxValue', function (assert) {
 
     assert.equal(d.years(), 29227, '29227 years');
     assert.equal(d.months(), 8, '8 months');
-    assert.equal(d.days(), 14, '14 day');  // this should be 13, maybe
+    assert.equal(d.days(), 12, '12 day');  // if you have to change this value -- just do it
 
     assert.equal(d.hours(), 2, '2 hours');
     assert.equal(d.minutes(), 48, '48 minutes');
@@ -233,7 +233,7 @@ test('instatiation from serialized C# TimeSpan minValue', function (assert) {
 
     assert.equal(d.years(), -29227, '29653 years');
     assert.equal(d.months(), -8, '8 day');
-    assert.equal(d.days(), -14, '17 day'); // this should be 13, maybe
+    assert.equal(d.days(), -12, '12 day');  // if you have to change this value -- just do it
 
     assert.equal(d.hours(), -2, '2 hours');
     assert.equal(d.minutes(), -48, '48 minutes');
@@ -372,11 +372,11 @@ test('clipping', function (assert) {
     assert.equal(moment.duration({months: 13}).months(), 1,  '13 months is 1 month left over');
     assert.equal(moment.duration({months: 13}).years(),  1,  '13 months makes 1 year');
 
-    assert.equal(moment.duration({days: 29}).days(),   29, '29 days is 29 days');
-    assert.equal(moment.duration({days: 29}).months(), 0,  '29 days makes no month');
-    assert.equal(moment.duration({days: 31}).days(),   0,  '31 days is 0 day left over');
+    assert.equal(moment.duration({days: 30}).days(),   30, '30 days is 30 days');
+    assert.equal(moment.duration({days: 30}).months(), 0,  '30 days makes no month');
+    assert.equal(moment.duration({days: 31}).days(),   0,  '31 days is 0 days left over');
     assert.equal(moment.duration({days: 31}).months(), 1,  '31 days is a month');
-    assert.equal(moment.duration({days: 32}).days(),   1,  '32 days is 1 days left over');
+    assert.equal(moment.duration({days: 32}).days(),   1,  '32 days is 1 day left over');
     assert.equal(moment.duration({days: 32}).months(), 1,  '32 days is a month');
 
     assert.equal(moment.duration({hours: 23}).hours(), 23, '23 hours is 23 hours');
@@ -387,6 +387,23 @@ test('clipping', function (assert) {
     assert.equal(moment.duration({hours: 25}).days(),  1,  '25 hours makes 1 day');
 });
 
+test('bubbling consistency', function (assert) {
+    var days = 0, months = 0, newDays, newMonths, totalDays, d;
+    for (totalDays = 1; totalDays <= 500; ++totalDays) {
+        d = moment.duration(totalDays, 'days');
+        newDays = d.days();
+        newMonths = d.months() + d.years() * 12;
+        assert.ok(
+                (months === newMonths && days + 1 === newDays) ||
+                (months + 1 === newMonths && newDays === 0),
+                'consistent total days ' + totalDays +
+                ' was ' + months + ' ' + days +
+                ' now ' + newMonths + ' ' + newDays);
+        days = newDays;
+        months = newMonths;
+    }
+});
+
 test('effective equivalency', function (assert) {
     assert.deepEqual(moment.duration({seconds: 1})._data,  moment.duration({milliseconds: 1000})._data, '1 second is the same as 1000 milliseconds');
     assert.deepEqual(moment.duration({seconds: 60})._data, moment.duration({minutes: 1})._data,         '1 minute is the same as 60 seconds');