Skip to content

Local date range #189

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

Closed
wants to merge 60 commits into from
Closed

Conversation

PeterAttardo
Copy link
Contributor

This pull request enables Kotlin range and progression semantics with the LocalDate and DatePeriod classes. Its implementation borrows from the patterns used for IntRange.

Example:

val start = LocalDate.parse("2020-01-01")
val end = LocalDate.parse("2020-12-31")

// calls function for each day of the year in 2020
(start..end).forEach {
    processDayOf2020(it)
}

// calls function for each day of the year in 2020, ordered in reverse
(end downTo start).forEach {
   processDayOf2020(it)
}

// calls function for every other day of the year in 2020
(start..end step DatePeriod(days=2)).forEach {
   processDayOf2020(it)
}

@dkhalanskyjb
Copy link
Collaborator

We have a similar request, but for Instant: #34
Could you please describe why this is needed? Either in the issue I linked if your use cases are similar to the ones listed there, or in a new issue.

@boswelja
Copy link

@dkhalanskyjb I've had a use case for this, though it's more of a convenience having a dedicated LocalDateRange compared to a ClosedRange<LocalDate>. I'm building a calendar library that allows consumers to update a particular range of dates on the screen, the intended use case for this is to allow changing the rendered content in chunks.

A few other use cases off the top of my head:

  • Representing a selected date range
  • Using plus/minus + forEach in conjunction to iterate over a large range in chunks (granted this could be achieved by having a list of dates and chunking that, but if you've only got a start and end date this could be more efficient)

Copy link
Collaborator

@dkhalanskyjb dkhalanskyjb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PeterAttardo
Copy link
Contributor Author

Something I came across that's not exactly an issue in the library, but is a usability question when using the library in IntelliJ that I'm wondering if there's a good answer for.

Because Kotlin automatically imports kotlin.ranges.* into every file, that means that the extension function fun <T : Comparable<T>> T.rangeTo(that: T): ClosedRange<T> is by-default available for all code. LocalDate implements Comparable<LocalDate> and so if you type the following:

(localDate1..localDate2).forEach {
    // some code in here
}

localDate1..localDate2 will not display an Unresolved reference warning due to a failure to import kotlinx.datetime.rangeTo, but will happily use kotlin.ranges.rangeTo and return a ClosedRange<LocalDate> instead. Because ClosedRange<LocalDate> is not LocalDateRange, all of our nice progression methods are unavailable, and we get an Unresolved reference warning on forEach instead.

This is easily solved by importing kotlinx.datetime.rangeTo, but IntelliJ is unable to recognize this as the solution, because from its perspective there is no issue resolving the rangeTo operator, just an issue with calling non-existing methods on ClosedRange<LocalDate>. This means users will have to be aware of this pitfall, and manually type in the import in every file they want to use the range operator with LocalDate.

@dkhalanskyjb
Copy link
Collaborator

@PeterAttardo, maybe this issue can be solved if rangeTo is a member method of LocalDate? Good job noticing this problem!

@PeterAttardo
Copy link
Contributor Author

@dkhalanskyjb I was hoping not to have to touch the LocalDate class, but that does seem like the cleanest solution. I have added the range operators to LocalDate, but because expect classes don't allow default implementations, I have moved the implementation logic into static methods in LocalDateRange. That can be cleaned up in the future if default implementations are ever allowed.

Copy link
Collaborator

@dkhalanskyjb dkhalanskyjb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compilation fails for me, and when I fix it, the getSize test fails.

Copy link
Collaborator

@dkhalanskyjb dkhalanskyjb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the small naming issue, I think it's good to go!

dkhalanskyjb pushed a commit that referenced this pull request Apr 28, 2025
@dkhalanskyjb
Copy link
Collaborator

Thank you a lot for the hard work! Merged manually in d15ec21 to squash this into one commit.

@lukellmann
Copy link
Contributor

I noticed that LocalDateProgression.random doesn't throw the documented exception.

@dkhalanskyjb
Copy link
Collaborator

@lukellmann, thanks, added the fix to the year-month branch (which, too, adds a random function): 8c04d69

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

Successfully merging this pull request may close these issues.

5 participants