Implement YearMonth #457
Implement YearMonth #457
Conversation
dkhalanskyjb
force-pushed
the
year-month
branch
2 times, most recently
from
a58f920 to
c42a067
Compare
dkhalanskyjb
force-pushed
the
year-month
branch
2 times, most recently
from
112e847 to
23b69c8
Compare
|
About time we get this added. You need to resolve merge conflicts |
|
You are right! Thanks for the advice! |
dkhalanskyjb
force-pushed
the
year-month
branch
from
23b69c8 to
186a5fc
Compare
| * | ||
| * @sample kotlinx.datetime.test.samples.YearMonthSamples.days | ||
| */ | ||
| // val days: LocalDateRange get() = firstDay..lastDay // no ranges yet |
There was a problem hiding this comment.
What is the reason for commenting this out? Seems useful to me.
There was a problem hiding this comment.
The reason is that LocalDateRange is not in master. See the description of the pull request.
| * | ||
| * @sample kotlinx.datetime.test.samples.YearMonthSamples.days | ||
| */ | ||
| // val days: LocalDateRange get() = firstDay..lastDay // no ranges yet |
There was a problem hiding this comment.
Another useful range would be an open end range of LocalDateTime that is midnight on the first day of the given month until midnight on the first day of the next month
There was a problem hiding this comment.
There are no ranges of LocalDateTime semantically, because due to DST transitions, sometimes, "earlier" local date-times follow the "later" ones, and some local date-times don't exist at all.
| * @throws DateTimeArithmeticException if the result exceeds the boundaries of [YearMonth]. | ||
| * @sample kotlinx.datetime.test.samples.YearMonthSamples.minusMonth | ||
| */ | ||
| public fun YearMonth.minusMonth(): YearMonth = minus(1, DateTimeUnit.MONTH) |
There was a problem hiding this comment.
These 4 functions should be plural and accept an int parameter that defaults to 1 as in:
public fun YearMonth.minusMonths(months: Int = 1): YearMonth = minus(months, DateTimeUnit.MONTH)
There was a problem hiding this comment.
In our research, we haven't found cases where the value was anything other than 1, so the extra flexibility is pointless. You have a more general plus for this case if 1 doesn't suit your needs.
core/common/src/YearMonth.kt
Outdated
| * | ||
| * @sample kotlinx.datetime.test.samples.YearMonthSamples.firstAndLastDay | ||
| */ | ||
| public val lastDay: LocalDate get() = onDay(numberOfDays) |
There was a problem hiding this comment.
You should add contains methods for LocalDate and LocalDateTime as in:
public fun contains(date: LocalDate): Boolean = date in firstDay..lastDay
Also helpful would be intersects functions for ranges to determine if this year month has any overlap with another range. This should include the cases of ClosedRange<LocalDate> and OpenEndRange<LocalDateTime>.
public fun intersects(dateRange: ClosedRange<LocalDate>): Boolean = ...
public fun intersects(dateTimeRange: OpenEndRange<LocalDateTime>): Boolean = ...
There was a problem hiding this comment.
Nope, we've decided that to interpret the YearMonth as a range, you'd use the days function (currently commented out).
dkhalanskyjb
force-pushed
the
year-month
branch
from
186a5fc to
9e89cb2
Compare
dkhalanskyjb
force-pushed
the
year-month
branch
2 times, most recently
from
48e41f2 to
8929852
Compare
dkhalanskyjb
force-pushed
the
year-month
branch
from
8929852 to
77c4d23
Compare
Fixes #168
Fixes #184
Related PRs:
YearMonth.days: LocalDateRangeand ranges ofYearMonthvalues.