Added support for nextDate and enumerateDates to Calendar#93
Added support for nextDate and enumerateDates to Calendar#93marcprux merged 34 commits intoskiptools:mainfrom
Conversation
| var previouslyReturnedMatchDate: Date? = nil | ||
| var iterations = -1 | ||
|
|
||
| repeat { |
There was a problem hiding this comment.
This STOP_EXHAUSTIVE_SEARCH_AFTER_MAX_ITERATIONS approach seems rather heavy-handed to me. I assume there's no equivalent function in Java's Calendar logic?
How does Foundation's Calendar do it?
There was a problem hiding this comment.
As far as I know, Java's Calendar does not have a direct native equivalent to Apple's enumerateDates function which is why I decided to base the implementation on the logic found in the Swift Foundation library.
I agree that this approach seems heavy-handed for preventing infinite loops. However, the Swift Foundation implements it in exactly the same way (see: https://github.com/swiftlang/swift-foundation/blob/main/Sources/FoundationEssentials/Calendar/Calendar_Enumerate.swift#L472C9-L472C62).
There was a problem hiding this comment.
Quick update, I haven't forgotten about this, but I'm still struggling with the size and complexity of the code this PR adds. I've been wondering if there might be some alternative, like internally using java.time.chrono if it offers some similar feature (disclosure: I have no idea if it does).
If there is really no other alternative solution, and if it isn't possible to get the size of the PR down, then I think we can go ahead and merge it since the test coverage is pretty good.
There was a problem hiding this comment.
Hi @marcprux,
thanks for your feedback. I completely understand your concerns about the size and complexity of this PR.
I’ve carefully reviewed the available Java classes, but unfortunately, I couldn’t find any native equivalent to Calendar.nextDate or Calendar.enumerateDates in Java that fully replicates the specific behavior found in Swift. Theoretically, we could implement this ourselves using java.time.temporal.TemporalAdjuster, but this approach would still require significant custom logic to handle all edge cases, such as week-of-month transitions and varying search directions.
Therefore, to minimize the risk associated with a full reimplementation, I’ve decided to adapt the core logic directly from the official Swift Foundation repository. While a reimplementation might have resulted in fewer lines of code, it would significantly increase the likelihood of introducing behavioral differences between Android and iOS.
Since Skip's goal is to provide a reliable and equivalent experience to Swift Foundation, I believe that prioritizing behavioral parity over code conciseness is the most robust and maintainable approach. Therefore, I recommend moving forward with this implementation and merging the PR.
marcprux
left a comment
There was a problem hiding this comment.
I still have reservations about the sheer size of this PR, but since it doesn't seem like there's any other way to support the equivalent Foundation API, and since the changes are almost purely additive, I think we can go ahead and merge this once CI passes.
|
Hi @marcprux, The CI has now passed too. The previous failure was due to a minor issue with a non-deterministic date within one test used in the runner environment. I've now resolved this by using a fixed date instead. That said, everything should be ready for merge now 😊 |
|
Thanks for all the hard work! This fills in a significant gap in Skip Lite's Foundation support. |
This PR extends the
Calendarclass to include support fornextDateandenumerateDates.The core logic was primarily adapted from the Swift Foundation repository to ensure parity with the native Swift behavior and minimize the risk of logic errors inherent in a full reimplementation. Since Skip currently only supports the Gregorian calendar, non-essential special cases were removed (e.g., lunar calendar offsets).
To ensure stability, I added several unit tests covering ranges, intervals, and enumeration. However, due to the complexity of calendar edge cases, it's possible that I overlooked a scenario.
Therefore, if you notice anaything, please let me know! 😊
Thank you for contributing to the Skip project! Please use this space to describe your change and add any labels (bug, enhancement, documentation, etc.) to help categorize your contribution.
Please review the contribution guide at https://skip.dev/docs/contributing/ for advice and guidance on making high-quality PRs.
Skip Pull Request Checklist:
swift testI used AI for assistance in creating additional unit tests for the calendar components and for consulting on Java-specific Calendar behaviors. Verification was done by manually reviewing all test cases, adjusting range expectations to match platform-specific constants, and ensuring the tests pass on both environments.