Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add LocalDateRange::toPeriod #51

Closed
BenMorel opened this issue May 29, 2022 · 4 comments
Closed

Add LocalDateRange::toPeriod #51

BenMorel opened this issue May 29, 2022 · 4 comments
Milestone

Comments

@BenMorel
Copy link
Member

BenMorel commented May 29, 2022

Following #48, in addition to LocalDateRange::getDuration, it would be useful to add a LocalDateRange::toPeriod() method.

There are several ways to implement this method. Take for example 2021-03-28/2022-04-01, we can represent it as either:

  • 1 year, 1 month, -27 days
  • 1 year, 0 month, 4 days
  • 0 year, 0 month, 369 days

Not sure which one we should choose here. The first one has a negative part for a non-negative range, so is probably not that intuitive; the third one is probably not what you would expect either, and one may argue that if you just want a raw number of days, you may use getDuration() instead. So at first glance, I'd go with the second option.

Another solution could be to offer several methods, but this can add to confusion.

I can see that Java's threeten-extra has a toPeriod method, it would be nice to see which way they followed.

Cc @solodkiy

@solodkiy
Copy link
Contributor

I am not really fan of this Period concept. Never used it in my production.
I personally could imagine use of convert 2021-03-28/2022-04-01 to Period 369 days, but 1 year, 0 month, 4 days.. what should we use it for?

@BenMorel
Copy link
Member Author

BenMorel commented May 30, 2022

Period is definitely useful to represent something like +6 months, that you cannot represent with a Duration, that only works with days.

In the case of LocalDateRange::toPeriod(), one use case I can think of is for display purposes, for example to display a person's age, in which case I think 1 year and 4 days is what you would expect here.

@BenMorel BenMorel added this to the 0.4.1 milestone May 30, 2022
@BenMorel
Copy link
Member Author

BenMorel commented Jun 5, 2022

Just tried it on Java, the output of the example above is P1Y4D, that is, 1 year and 4 days 👍

And the implementation just uses Period::between(), which we have already implemented, so the implementation will be trivial.

@BenMorel BenMorel changed the title Add LocalDateRange::getPeriod Add LocalDateRange::toPeriod Jun 5, 2022
@BenMorel
Copy link
Member Author

BenMorel commented Jun 5, 2022

Implemented in 213a4ba.

@BenMorel BenMorel closed this as completed Jun 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants