search

moment / luxon

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

Add `Duration.min()` and `Duration.max()` #1527

Open jirutka opened 12 months ago

jirutka commented 12 months ago

Can you please add static methods min and max to the Duration, analogous to the methods in DateTime?

shashankbhat10 commented 11 months ago

Hey @icambron,

If you are ok with adding these methods to the Duration class, can I be assigned this issue? I have made some changes regarding this and can raise a PR for it.

diesieben07 commented 11 months ago

This definitely is not trivial. The problem is that durations by default use "casual" conversion rules, which can make it so this comparison would not be reflexive or transitive, depending on which unit you use for comparison. For example: 29 days > 4 weeks, using days 4 weeks == 1 month, using weeks 29 days < 1 month, using days

So, 29 days is both larger and smaller than 1 month, considering the above logic.

Also: 4 weeks == 1 month, using weeks; but 4 weeks < 1 month, using days (because 28 days < 30 days)

This gets even more complicated if you have multiple (even different) units on both sides. So really, the user needs to tell which unit to use for the comparison. And even then they might not get the result they want.

shashankbhat10 commented 11 months ago

I get what you are saying. I think could be done after any resolution of #1514, right?

diesieben07 commented 11 months ago

No, this is an inherent property of the casual conversion matrix that Durations use by default. You can read more about it in the documentation.