moment / luxon

⏱ A library for working with dates and times in JS
https://moment.github.io/luxon
MIT License
15.41k stars 729 forks source link

Shouldn't Duration::equals(other: Duration): boolean check the value for equality? #1652

Closed ramirezsandin closed 2 months ago

ramirezsandin commented 2 months ago

Is your feature request related to a problem? Please describe. Current implementation of equality is defined as: Two Durations are equal if they have the same units and the same values for each unit.

Describe the solution you'd like I think equality should be given by its value (if Duration.toMillis() or Duration.valueOf() are equals) instead of checking for the value of each unit.

Additional context example, it is weird that this is not true: Duration.fromObject({ hour: 1 }).equals(Duration.fromObject({ minutes: 60 })

Best, Jorge

icambron commented 2 months ago

No, for a couple reasons:

  1. Many conversions aren't perfect: months aren't all the same length, for example, and neither are years. Implicitly converting between units would not only be confusing, but would need a conversionAccuracy setting
  2. Even for units with incontrovertible conversions, this would be weird thing to do, and doesn't reflect the usual semantics of object equality, because you can't treat those objects interchangeably; their accessors return very different things. The whole reason durations have different units is so that programs can treat them differently.

Fortunately, if you want to compare the milliseconds represented by the durations instead of the exact units and their quantities, that's very easy:

Duration.fromObject({ hours: 1 }).toMillis() == Duration.fromObject({ minutes: 60 }).toMillis() //=> true

Note that not only does it work, it lines up perfectly with your intentions: you want to "boil down" the durations to just milliseconds for the purpose of comparison, eliding the units that the duration represents, and that's exactly what the code says it's doing.

ramirezsandin commented 1 month ago

No, for a couple reasons:

  1. Many conversions aren't perfect: months aren't all the same length, for example, and neither are years. Implicitly converting between units would not only be confusing, but would need a conversionAccuracy setting
  2. Even for units with incontrovertible conversions, this would be weird thing to do, and doesn't reflect the usual semantics of object equality, because you can't treat those objects interchangeably; their accessors return very different things. The whole reason durations have different units is so that programs can treat them differently.

Fortunately, if you want to compare the milliseconds represented by the durations instead of the exact units and their quantities, that's very easy:

Duration.fromObject({ hours: 1 }).toMillis() == Duration.fromObject({ minutes: 60 }).toMillis() //=> true

Note that not only does it work, it lines up perfectly with your intentions: you want to "boil down" the durations to just milliseconds for the purpose of comparison, eliding the units that the duration represents, and that's exactly what the code says it's doing.

Completely agree with your arguments, durations would not be accurate if they are created from objects with those units (at least if they are not relative to a particular datetime).

Thank you for your time.

Best regards, Jorge