Skip to content

Commit

Permalink
difference: throw out-of-order; zap Time 12h limit
Browse files Browse the repository at this point in the history
Fixes #663. Changes `difference()` to throw if `other`is
larger than `this`. Goal is encourage feedback about long-term solution:
option 1 - previous behavior: `difference()` returns an absolute value
option 2 - behavior in this commit: throw if other is larger
option 3 - negative durations (#558)

Tests are updated to make it easy to choose any of the three options.
Tests for option 1 are commented out in case we want to revert.
The rest of test changes were reversing arguments so that choosing any
of the options will break few tests.

Because `difference` is now unidirectional, it doesn't make sense to
restrict `Time.prototype.difference` to 12 hours max. Fixes #597.
  • Loading branch information
justingrant committed Jun 12, 2020
1 parent d23bc2d commit 6ff4c9a
Show file tree
Hide file tree
Showing 24 changed files with 124 additions and 89 deletions.
16 changes: 8 additions & 8 deletions docs/absolute.md
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ Temporal.now.absolute().minus(oneDay);
**Returns:** a `Temporal.Duration` representing the difference between `absolute` and `other`.

This method computes the difference between the two times represented by `absolute` and `other`, and returns it as a `Temporal.Duration` object.
The difference is always positive, no matter the order of `absolute` and `other`, because `Temporal.Duration` objects cannot represent negative durations.
A `RangeError` will be thrown if `other` is later than `absolute`, because `Temporal.Duration` objects cannot represent negative durations.

The `largestUnit` option controls how the resulting duration is expressed.
The returned `Temporal.Duration` object will not have any nonzero fields that are larger than the unit in `largestUnit`.
Expand All @@ -312,25 +312,25 @@ Example usage:
```js
startOfMoonMission = Temporal.Absolute.from('1969-07-16T13:32:00Z');
endOfMoonMission = Temporal.Absolute.from('1969-07-24T16:50:35Z');
missionLength = startOfMoonMission.difference(endOfMoonMission, { largestUnit: 'days' });
// => P8DT3H18M35S
endOfMoonMission.difference(startOfMoonMission, { largestUnit: 'days' });
missionLength = endOfMoonMission.difference(startOfMoonMission, { largestUnit: 'days' });
// => P8DT3H18M35S
startOfMoonMission.difference(endOfMoonMission, { largestUnit: 'days' });
// => throws RangeError
missionLength.toLocaleString();
// example output: '8 days 3 hours 18 minutes 35 seconds'

// A billion (10^9) seconds since the epoch in different units
epoch = new Temporal.Absolute(0n);
billion = Temporal.Absolute.fromEpochSeconds(1e9);
epoch.difference(billion); // => PT1000000000S
epoch.difference(billion, { largestUnit: 'hours' }) // => PT277777H46M40S
epoch.difference(billion, { largestUnit: 'days' }) // => P11574DT1H46M40S
billion.difference(epoch); // => PT1000000000S
billion.difference(epoch, { largestUnit: 'hours' }) // => PT277777H46M40S
billion.difference(epoch, { largestUnit: 'days' }) // => P11574DT1H46M40S

// If you really need to calculate the difference between two Absolutes
// in years, you can eliminate the ambiguity by choosing your starting
// point explicitly. For example, using the corresponding UTC date:
utc = Temporal.TimeZone.from('UTC');
epoch.inTimeZone(utc).difference(billion.inTimeZone(utc), { largestUnit: 'years' });
billion.inTimeZone(utc).difference(epoch.inTimeZone(utc), { largestUnit: 'years' });
// => P31Y8M8DT1H46M40S
```

Expand Down
4 changes: 2 additions & 2 deletions docs/calendar.md
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,12 @@ For example:
```javascript
d1 = Temporal.Date.from('2020-07-29').withCalendar('chinese');
d2 = Temporal.Date.from('2020-08-29').withCalendar('chinese');
d1.difference(d2, { largestUnit: 'months' }) // => P1M2D
d2.difference(d1, { largestUnit: 'months' }) // => P1M2D

// same result, but calling the method directly:
Temporal.Calendar.from('chinese').difference(
Temporal.Date.from('2020-07-29'),
Temporal.Date.from('2020-08-29'),
Temporal.Date.from('2020-07-29'),
{ largestUnit: 'months' }
) // => P1M2D
```
Expand Down
4 changes: 2 additions & 2 deletions docs/cookbook/futureDateForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ function englishPlural(n, singular, plural) {
if (futuredateParam !== null) {
const futureDate = Temporal.Date.from(futuredateParam);
const today = Temporal.now.date();
const until = today.difference(futureDate, { largestUnit: 'days' });
const untilMonths = today.difference(futureDate, { largestUnit: 'months' });
const until = futureDate.difference(today, { largestUnit: 'days' });
const untilMonths = futureDate.difference(today, { largestUnit: 'months' });

const dayString = englishPlural(until.days, 'day', 'days');
const monthString =
Expand Down
2 changes: 1 addition & 1 deletion docs/cookbook/getElapsedDurationSinceInstant.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
*/
function getElapsedDurationSinceInstant(then, now, largestUnit = 'days') {
const sign = Temporal.Absolute.compare(now, then) < 0 ? '-' : '+';
const duration = now.difference(then, { largestUnit });
const duration = sign === '-' ? then.difference(now, { largestUnit }) : now.difference(then, { largestUnit });
return { sign, duration };
}

Expand Down
2 changes: 1 addition & 1 deletion docs/cookbook/getTripDurationInHrMinSec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
function getTripDurationInHrMinSec(parseableDeparture, parseableArrival) {
const departure = Temporal.Absolute.from(parseableDeparture);
const arrival = Temporal.Absolute.from(parseableArrival);
return departure.difference(arrival, { largestUnit: 'hours' });
return arrival.difference(departure, { largestUnit: 'hours' });
}

const flightTime = getTripDurationInHrMinSec(
Expand Down
7 changes: 4 additions & 3 deletions docs/date.md
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ date.minus({ months: 1 }, { disambiguation: 'reject' }) // => throws
**Returns:** a `Temporal.Duration` representing the difference between `date` and `other`.

This method computes the difference between the two dates represented by `date` and `other`, and returns it as a `Temporal.Duration` object.
The difference is always positive, no matter the order of `date` and `other`, because `Temporal.Duration` objects cannot represent negative durations.
A `RangeError` will be thrown if `other` is later than `date`, because `Temporal.Duration` objects cannot represent negative durations.

The `largestUnit` option controls how the resulting duration is expressed.
The returned `Temporal.Duration` object will not have any nonzero fields that are larger than the unit in `largestUnit`.
Expand All @@ -351,10 +351,11 @@ Unlike other Temporal types, hours and lower are not allowed, because the data m

Usage example:
```javascript
date = Temporal.Date.from('2006-08-24');
other = Temporal.Date.from('2019-01-31');
date = Temporal.Date.from('2019-01-31');
other = Temporal.Date.from('2006-08-24');
date.difference(other) // => P4543D
date.difference(other, { largestUnit: 'years' }) // => P12Y5M7D
other.difference(date, { largestUnit: 'years' }) // => throws RangeError

// If you really need to calculate the difference between two Dates in
// hours, you can eliminate the ambiguity by explicitly choosing the
Expand Down
15 changes: 8 additions & 7 deletions docs/datetime.md
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ dt.minus({ months: 1 }) // => throws
**Returns:** a `Temporal.Duration` representing the difference between `datetime` and `other`.

This method computes the difference between the two times represented by `datetime` and `other`, and returns it as a `Temporal.Duration` object.
The difference is always positive, no matter the order of `datetime` and `other`, because `Temporal.Duration` objects cannot represent negative durations.
A `RangeError` will be thrown if `other` is later than `datetime`, because `Temporal.Duration` objects cannot represent negative durations.

The `largestUnit` option controls how the resulting duration is expressed.
The returned `Temporal.Duration` object will not have any nonzero fields that are larger than the unit in `largestUnit`.
Expand All @@ -408,15 +408,16 @@ Usage example:
```javascript
dt1 = Temporal.DateTime.from('1995-12-07T03:24:30.000003500');
dt2 = Temporal.DateTime.from('2019-01-31T15:30');
dt1.difference(dt2); // => P8456DT12H5M29.999996500S
dt1.difference(dt2), { largestUnit: 'years' }) // => P23Y1M24DT12H5M29.999996500S
dt2.difference(dt1); // => P8456DT12H5M29.999996500S
dt2.difference(dt1), { largestUnit: 'years' }) // => P23Y1M24DT12H5M29.999996500S
dt1.difference(dt2), { largestUnit: 'years' }) // => throws RangeError

// Months and years can be different lengths
[jan1, feb1, mar1] = [1, 2, 3].map(month => Temporal.DateTime.from({year: 2020, month, day: 1}));
jan1.difference(feb1); // => P31D
jan1.difference(feb1, { largestUnit: 'months' }); // => P1M
feb1.difference(mar1); // => P29D
feb1.difference(mar1, { largestUnit: 'months' }); // => P1M
feb1.difference(jan1); // => P31D
feb1.difference(jan1, { largestUnit: 'months' }); // => P1M
mar1.difference(feb1); // => P29D
mar1.difference(feb1, { largestUnit: 'months' }); // => P1M
```

### datetime.**equals**(_other_: Temporal.DateTime) : boolean
Expand Down
18 changes: 8 additions & 10 deletions docs/time.md
Original file line number Diff line number Diff line change
Expand Up @@ -245,23 +245,21 @@ time.minus({ minutes: 5, nanoseconds: 800 }) // => 19:34:09.068345405
**Returns:** a `Temporal.Duration` representing the difference between `time` and `other`.

This method computes the difference between the two times represented by `time` and `other`, and returns it as a `Temporal.Duration` object.
The difference is always positive, and always 12 hours or less, no matter the order of `time` and `other`, because `Temporal.Duration` objects cannot represent negative durations.
A `RangeError` will be thrown if `other` is later than `time`, because `Temporal.Duration` objects cannot represent negative durations.

The `largestUnit` parameter controls how the resulting duration is expressed.
The returned `Temporal.Duration` object will not have any nonzero fields that are larger than the unit in `largestUnit`.
A difference of two hours will become 7200 seconds when `largestUnit` is `"seconds"`, for example.
However, a difference of 30 seconds will still be 30 seconds even if `largestUnit` is `"hours"`.
A difference of two hours will become 7200 seconds when `largestUnit` is `'seconds'`, for example.
However, a difference of 30 seconds will still be 30 seconds even if `largestUnit` is `'hours'`.

The default largest unit in the result is technically days, for consistency with other Temporal types' `difference` methods.
However, since this method never returns any duration longer than 12 hours, largest units of years, months, or days, are by definition treated as hours.
The default largest unit in the result is technically `'days'`, for consistency with other Temporal types' `difference` methods.
However, since time differences are always shorter than one day, largest units of `'years'`, `'months'`, or `'days'` are treated as `'hours'`.

Usage example:
```javascript
time = Temporal.Time.from('19:39:09.068346205');
time.difference(Temporal.Time.from('20:13:20.971398099')) // => PT34M11.903051894S

// The difference is always less than 12 hours, crossing midnight if needed
Temporal.Time.from('01:00').difference(Temporal.Time.from('23:00')) // => P2H
time = Temporal.Time.from('20:13:20.971398099');
time.difference(Temporal.Time.from('19:39:09.068346205')) // => PT34M11.903051894S
time.difference(Temporal.Time.from('22:39:09.068346205')) // => throws RangeError
```

### time.**equals**(_other_: Temporal.Time) : boolean
Expand Down
3 changes: 2 additions & 1 deletion docs/yearmonth.md
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ ym.minus({years: 20, months: 4}) // => 1999-02
**Returns:** a `Temporal.Duration` representing the difference between `yearMonth` and `other`.

This method computes the difference between the two months represented by `yearMonth` and `other`, and returns it as a `Temporal.Duration` object.
The difference is always positive, no matter the order of `yearMonth` and `other`, because `Temporal.Duration` objects cannot represent negative durations.
A `RangeError` will be thrown if `other` is later than `yearMonth`, because `Temporal.Duration` objects cannot represent negative durations.

The `largestUnit` option controls how the resulting duration is expressed.
The returned `Temporal.Duration` object will not have any nonzero fields that are larger than the unit in `largestUnit`.
Expand All @@ -299,6 +299,7 @@ ym = Temporal.YearMonth.from('2019-06');
other = Temporal.YearMonth.from('2006-08');
ym.difference(other) // => P12Y10M
ym.difference(other, { largestUnit: 'months' }) // => P154M
other.difference(ym, { largestUnit: 'months' }) // => throws RangeError

// If you really need to calculate the difference between two YearMonths
// in days, you can eliminate the ambiguity by explicitly choosing the
Expand Down
7 changes: 4 additions & 3 deletions polyfill/lib/absolute.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,10 @@ export class Absolute {
if (!ES.IsTemporalAbsolute(other)) throw new TypeError('invalid Absolute object');
const largestUnit = ES.ToLargestTemporalUnit(options, 'seconds', ['years', 'months', 'weeks']);

const [one, two] = [this, other].sort(Absolute.compare);
const onens = GetSlot(one, EPOCHNANOSECONDS);
const twons = GetSlot(two, EPOCHNANOSECONDS);
const comparison = Absolute.compare(this, other);
if (comparison < 0) throw new RangeError('other instance cannot be larger than `this`');
const onens = GetSlot(other, EPOCHNANOSECONDS);
const twons = GetSlot(this, EPOCHNANOSECONDS);
const diff = twons.minus(onens);

const ns = +diff.mod(1e3);
Expand Down
5 changes: 3 additions & 2 deletions polyfill/lib/date.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,9 @@ export class Date {
if (!ES.IsTemporalDate(this)) throw new TypeError('invalid receiver');
if (!ES.IsTemporalDate(other)) throw new TypeError('invalid Date object');
const largestUnit = ES.ToLargestTemporalUnit(options, 'days', ['hours', 'minutes', 'seconds']);
const [smaller, larger] = [this, other].sort(Date.compare);
const { years, months, weeks, days } = ES.DifferenceDate(smaller, larger, largestUnit);
const comparison = Date.compare(this, other);
if (comparison < 0) throw new RangeError('other instance cannot be larger than `this`');
const { years, months, weeks, days } = ES.DifferenceDate(other, this, largestUnit);
const Duration = GetIntrinsic('%Temporal.Duration%');
return new Duration(years, months, weeks, days, 0, 0, 0, 0, 0, 0);
}
Expand Down
11 changes: 6 additions & 5 deletions polyfill/lib/datetime.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -239,12 +239,13 @@ export class DateTime {
if (!ES.IsTemporalDateTime(this)) throw new TypeError('invalid receiver');
if (!ES.IsTemporalDateTime(other)) throw new TypeError('invalid DateTime object');
const largestUnit = ES.ToLargestTemporalUnit(options, 'days');
const [smaller, larger] = [this, other].sort(DateTime.compare);
const comparison = DateTime.compare(this, other);
if (comparison < 0) throw new RangeError('other instance cannot be larger than `this`');
let { deltaDays, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } = ES.DifferenceTime(
smaller,
larger
other,
this
);
let { year, month, day } = larger;
let { year, month, day } = this;
day += deltaDays;
({ year, month, day } = ES.BalanceDate(year, month, day));

Expand All @@ -253,7 +254,7 @@ export class DateTime {
dateLargestUnit = largestUnit;
}

let { years, months, weeks, days } = ES.DifferenceDate(smaller, { year, month, day }, dateLargestUnit);
let { years, months, weeks, days } = ES.DifferenceDate(other, { year, month, day }, dateLargestUnit);

({ days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } = ES.BalanceDuration(
days,
Expand Down
7 changes: 5 additions & 2 deletions polyfill/lib/time.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,10 @@ export class Time {
if (!ES.IsTemporalTime(this)) throw new TypeError('invalid receiver');
if (!ES.IsTemporalTime(other)) throw new TypeError('invalid Time object');
const largestUnit = ES.ToLargestTemporalUnit(options, 'hours');
const [earlier, later] = [this, other].sort(Time.compare);
let { hours, minutes, seconds, milliseconds, microseconds, nanoseconds } = ES.DifferenceTime(earlier, later);
const comparison = Time.compare(this, other);
if (comparison < 0) throw new RangeError('other instance cannot be larger than `this`');
let { hours, minutes, seconds, milliseconds, microseconds, nanoseconds } = ES.DifferenceTime(other, this);
/*
if (hours >= 12) {
hours = 24 - hours;
minutes *= -1;
Expand All @@ -178,6 +180,7 @@ export class Time {
microseconds *= -1;
nanoseconds *= -1;
}
*/
({ hours, minutes, seconds, milliseconds, microseconds, nanoseconds } = ES.BalanceDuration(
0,
hours,
Expand Down
7 changes: 4 additions & 3 deletions polyfill/lib/yearmonth.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,10 @@ export class YearMonth {
if (!ES.IsTemporalYearMonth(this)) throw new TypeError('invalid receiver');
if (!ES.IsTemporalYearMonth(other)) throw new TypeError('invalid YearMonth object');
const largestUnit = ES.ToLargestTemporalUnit(options, 'years', ['weeks', 'days', 'hours', 'minutes', 'seconds']);
const [one, two] = [this, other].sort(YearMonth.compare);
let years = two.year - one.year;
let months = two.month - one.month;
const comparison = YearMonth.compare(this, other);
if (comparison < 0) throw new RangeError('other instance cannot be larger than `this`');
let years = this.year - other.year;
let months = this.month - other.month;
if (months < 0) {
years -= 1;
months += 12;
Expand Down
7 changes: 4 additions & 3 deletions polyfill/test/absolute.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -423,9 +423,10 @@ describe('Absolute', () => {
describe('Absolute.difference works', () => {
const earlier = Absolute.from('1976-11-18T15:23:30.123456789Z');
const later = Absolute.from('2019-10-29T10:46:38.271986102Z');
const diff = earlier.difference(later);
it(`(${earlier}).difference(${later}) == (${later}).difference(${earlier})`, () =>
equal(`${later.difference(earlier)}`, `${diff}`));
const diff = later.difference(earlier);
// it(`(${earlier}).difference(${later}) == (${later}).difference(${earlier})`, () =>
// equal(`${later.difference(earlier)}`, `${diff}`));
it('throws if out of order', () => throws(() => earlier.difference(later), RangeError));
it(`(${earlier}).plus(${diff}) == (${later})`, () => assert(earlier.plus(diff).equals(later)));
it(`(${later}).minus(${diff}) == (${earlier})`, () => assert(later.minus(diff).equals(earlier)));
it("doesn't cast argument", () => {
Expand Down
17 changes: 9 additions & 8 deletions polyfill/test/date.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ describe('Date', () => {
equal(duration.nanoseconds, 0);
});
it('date.difference({ year: 2019, month: 11, day: 18 }, { largestUnit: "years" })', () => {
const duration = date.difference(Date.from({ year: 2019, month: 11, day: 18 }), { largestUnit: 'years' });
const later = Date.from({ year: 2019, month: 11, day: 18 });
const duration = later.difference(date, { largestUnit: 'years' });
equal(duration.years, 43);
equal(duration.months, 0);
equal(duration.weeks, 0);
Expand All @@ -182,12 +183,12 @@ describe('Date', () => {
const date1 = Date.from('2019-01-01');
const date2 = Date.from('2019-02-01');
const date3 = Date.from('2019-03-01');
equal(`${date1.difference(date2)}`, 'P31D');
equal(`${date2.difference(date3)}`, 'P28D');
equal(`${date2.difference(date1)}`, 'P31D');
equal(`${date3.difference(date2)}`, 'P28D');

const date4 = Date.from('2020-02-01');
const date5 = Date.from('2020-03-01');
equal(`${date4.difference(date5)}`, 'P29D');
equal(`${date5.difference(date4)}`, 'P29D');
});
it('takes days per year into account', () => {
const date1 = Date.from('2019-01-01');
Expand All @@ -196,10 +197,10 @@ describe('Date', () => {
const date4 = Date.from('2020-06-01');
const date5 = Date.from('2021-01-01');
const date6 = Date.from('2021-06-01');
equal(`${date1.difference(date3)}`, 'P365D');
equal(`${date3.difference(date5)}`, 'P366D');
equal(`${date2.difference(date4)}`, 'P366D');
equal(`${date4.difference(date6)}`, 'P365D');
equal(`${date3.difference(date1)}`, 'P365D');
equal(`${date5.difference(date3)}`, 'P366D');
equal(`${date4.difference(date2)}`, 'P366D');
equal(`${date6.difference(date4)}`, 'P365D');
});
const feb20 = Date.from('2020-02-01');
const feb21 = Date.from('2021-02-01');
Expand Down
5 changes: 4 additions & 1 deletion polyfill/test/datetime.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,12 @@ describe('DateTime', () => {
const earlier = DateTime.from('1976-11-18T15:23:30.123456789');
const later = DateTime.from('2019-10-29T10:46:38.271986102');
['years', 'months', 'weeks', 'days', 'hours', 'minutes', 'seconds'].forEach((largestUnit) => {
const diff = earlier.difference(later, { largestUnit });
const diff = later.difference(earlier, { largestUnit });
/*
it(`(${earlier}).difference(${later}, ${largestUnit}) == (${later}).difference(${earlier}, ${largestUnit})`, () =>
equal(`${later.difference(earlier, { largestUnit })}`, `${diff}`));
*/
it('throws if out of order', () => throws(() => earlier.difference(later), RangeError));
it(`(${earlier}).plus(${diff}) == (${later})`, () => earlier.plus(diff).equals(later));
it(`(${later}).minus(${diff}) == (${earlier})`, () => later.minus(diff).equals(earlier));
});
Expand Down
Loading

0 comments on commit 6ff4c9a

Please sign in to comment.