brick
Add LocalDateRange::toDuration, YearMonthRange::toDuration methods
Also we could add this method to YearMonthRange (looks reasonable), and Year, YearMonth, YearWeek (not so sure).
Apart from my comment, LGTM :+1:
Could you please consider adding LocalDateRange::getPeriod() as well in another PR? Although that one is less straightforward, as we have 2 ways to convert it to a period: calculating x years, y months & z days, or just 0 years, 0 months, and n days. Maybe we should even offer both ways? Let's start the discussion in #51.
YearMonthRange::getPeriod() would make sense, but not getDuration() as you cannot convert it to a number of days.
As for Year, YearMonth, YearWeek, they're not ranges, so I don't see how this could translate to a Duration or Period?
YearMonthRange::getPeriod() would make sense, but not getDuration() as you cannot convert it to a number of days.
Why not? YearMonthRange is inclusive. It include full first month, everything in between and full last month. So I think YearMonthRange include all days between first day of first range month and last day of last range month (include corner days). This approach allow us to convert YearMonthRange to LocalDateRange and to Duration.
Year is not really a range, but also could represent a whole year from first day to last.
I see. Let's start with this one and discuss the others in another PR.
Also, Java's threeten-extra has a toPeriod method, that we may replicate here; maybe we should name this one toDuration() for consistency then?
maybe we should name this one toDuration() for consistency then?
I am not sure we should fight for consistency with Java here.
Can you describe main logic of choosing between to and get methods?
I have no idea, but this project started as port of Java's ThreeTen API, so whenever we can keep the same API, I think we should do it for two reasons:
- there are many people behind this initiative, so they may have had this discussion before, and may have come to this consensus for a reason; to quote another contributor's comment:
(...) we can't possibly replicate the collective brainpower and empirical evidence that went into designing JSR 310 and so, where practical, I would suggest simply copying JSR 310's API
- this can make this project less confusing for people coming from a Java background
As for the logic behind this, what about:
get*()for fields that are part of the object (getYear(),getMonth(), ...)to*()for objects that are not part of it, but can be derived from it? (toDuration())
Though I'm sure you can find a counter-example in the codebase.
Though I'm sure you can find a counter-example in the codebase.
I will look for it, maybe it good point to do some rename for consistency then.
I finally chose toPeriod() in #51 for consistency with threeten-extra, so please let's use toDuration() here for consistency.
Though I'm sure you can find a counter-example in the codebase.
I will look for it, maybe it good point to do some rename for consistency then.
Thank you, that'd be great.
I think about time zones and Local Range duration. In some time zones with Daylight Saving Time duration between day start and day end is not so obvious.
For example Europe/London:
Sunday, 27 March 2022, 01:00:00 clocks were turned forward 1 hour to Sunday, 27 March 2022, 02:00:00 local daylight time instead.
So Duration of range 27 March 2022 to 28 March 2022 in London is 47h.
Should we allow pass TimeZone param to toDuration() method?
I agree that you need the timezone if you want to introduce a toDuration() method, as 24 hours per day would be an oversimplification here.
Maybe that's the reason why threeten-extra doesn't provide this method at all.
What's your use case for this method exactly?
What's your use case for this method exactly?
Actually I don't remember it anymore :) I will return when I do.