diff --git a/src/duration.js b/src/duration.js index 76a57b27..b725b59e 100644 --- a/src/duration.js +++ b/src/duration.js @@ -10,7 +10,6 @@ import { isUndefined, normalizeObject, roundTo, - signedFloor, } from "./impl/util.js"; import Settings from "./settings.js"; import DateTime from "./datetime.js"; @@ -127,27 +126,46 @@ function clone(dur, alts, clear = false) { return new Duration(conf); } -// this is needed since in some test cases it would return 0.9999999999999999 instead of 1 -function removePrecisionIssue(a) { - return Math.trunc(a * 1e3) / 1e3; -} - -// NB: mutates parameters -function convert(matrix, fromMap, fromUnit, toMap, toUnit) { - const conv = matrix[toUnit][fromUnit]; - const raw = fromMap[fromUnit] / conv; - const added = signedFloor(raw); - - toMap[toUnit] = removePrecisionIssue(toMap[toUnit] + added); - fromMap[fromUnit] = removePrecisionIssue(fromMap[fromUnit] - added * conv); +function durationToMillis(matrix, vals) { + let sum = vals.milliseconds ?? 0; + for (const unit of reverseUnits.slice(1)) { + if (vals[unit]) { + sum += vals[unit] * matrix[unit]["milliseconds"]; + } + } + return sum; } // NB: mutates parameters function normalizeValues(matrix, vals) { + // the logic below assumes the overall value of the duration is positive + // if this is not the case, factor is used to make it so + const factor = durationToMillis(matrix, vals) < 0 ? -1 : 1; + reverseUnits.reduce((previous, current) => { if (!isUndefined(vals[current])) { if (previous) { - convert(matrix, vals, previous, vals, current); + const previousVal = vals[previous] * factor; + const conv = matrix[current][previous]; + + // if (previousVal < 0): + // lower order unit is negative (e.g. { years: 2, days: -2 }) + // normalize this by reducing the higher order unit by the appropriate amount + // and increasing the lower order unit + // this can never make the higher order unit negative, because this function only operates + // on positive durations, so the amount of time represented by the lower order unit cannot + // be larger than the higher order unit + // else: + // lower order unit is positive (e.g. { years: 2, days: 450 } or { years: -2, days: 450 }) + // in this case we attempt to convert as much as possible from the lower order unit into + // the higher order one + // + // Math.floor takes care of both of these cases, rounding away from 0 + // if previousVal < 0 it makes the absolute value larger + // if previousVal >= it makes the absolute value smaller + const rollUp = Math.floor(previousVal / conv); + vals[current] += rollUp * factor; + vals[previous] -= rollUp * conv * factor; } return current; } else { @@ -581,13 +599,7 @@ export default class Duration { toMillis() { if (!this.isValid) return NaN; - let sum = this.values.milliseconds ?? 0; - for (let unit of reverseUnits.slice(1)) { - if (this.values[unit]) { - sum += this.values[unit] * this.matrix[unit]["milliseconds"]; - } - } - return sum; + return durationToMillis(this.matrix, this.values); } /** @@ -697,18 +709,22 @@ export default class Duration { /** * Reduce this Duration to its canonical representation in its current units. + * Assuming the overall value of the Duration is positive, this means: + * - excessive values for lower-order units are converted to higher order units (if possible, see first and second example) + * - negative lower-order units are converted to higher order units (there must be such a higher order unit, otherwise + * the overall value would be negative, see second example) + * + * If the overall value is negative, the result of this method is equivalent to `this.negate().normalize().negate()`. * @example Duration.fromObject({ years: 2, days: 5000 }).normalize().toObject() //=> { years: 15, days: 255 } + * @example Duration.fromObject({ days: 5000 }).normalize().toObject() //=> { days: 5000 } * @example Duration.fromObject({ hours: 12, minutes: -45 }).normalize().toObject() //=> { hours: 11, minutes: 15 } * @return {Duration} */ normalize() { if (!this.isValid) return this; const vals = this.toObject(); - if (this.valueOf() >= 0) { - normalizeValues(this.matrix, vals); - return clone(this, { values: vals }, true); - } - return this.negate().normalize().negate(); + normalizeValues(this.matrix, vals); + return clone(this, { values: vals }, true); } /** @@ -758,16 +774,12 @@ export default class Duration { own += vals[k]; } + // only keep the integer part for now in the hopes of putting any decimal part + // into a smaller unit later const i = Math.trunc(own); built[k] = i; accumulated[k] = (own * 1000 - i * 1000) / 1000; - // plus anything further down the chain that should be rolled up in to this - for (const down in vals) { - if (orderedUnits.indexOf(down) > orderedUnits.indexOf(k)) { - convert(this.matrix, vals, down, built, k); - } - } // otherwise, keep it in the wings to boil it later } else if (isNumber(vals[k])) { accumulated[k] = vals[k]; @@ -783,7 +795,8 @@ export default class Duration { } } - return clone(this, { values: built }, true).normalize(); + normalizeValues(this.matrix, built); + return clone(this, { values: built }, true); } /** diff --git a/src/impl/util.js b/src/impl/util.js index b934b105..51f756ea 100644 --- a/src/impl/util.js +++ b/src/impl/util.js @@ -124,10 +124,6 @@ export function parseMillis(fraction) { } } -export function signedFloor(number) { - return number > 0 ? Math.floor(number) : Math.ceil(number); -} - export function roundTo(number, digits, towardZero = false) { const factor = 10 ** digits, rounder = towardZero ? Math.trunc : Math.round;