Calendar Sequence API #322
Conversation
|
@swift-ci test |
parkera
force-pushed
the
parkera/calendar_sequence
branch
from
de8ae4b to
fc3f82c
Compare
| guard let startAt = previousResult else { | ||
| // We have finished | ||
| return nil | ||
| } | ||
|
|
||
| let next = calendar.date(byAdding: components, to: startAt, wrappingComponents: wrappingComponents) |
There was a problem hiding this comment.
This approach is susceptible to the "1+1≠2" problem in calendar math. By always adding from the previous result, we're allowing for drift to occur.
As a simple example, if the start date is "January 31" and the stride is "1 month", then the first addition will result in "February 28". The addition after that will result in "March 28", which has drifted from the likely expected value of "March 31".
The typical way to solve this is to always add from the start date, and to scale the value being added. So you start by adding one month, then two months, then three months etc; instead of adding one month every time.
This, however, gives rise to other questions about which units can be used as a stride.
DateComponents allows you to specify values that aren't relevant to addition, such as the .calendar, .timezone, .weekday, etc. It seems to me that the sequence and iterator initializers should include checks to fatalError() if the components include values that are illogical to use as a stride.
| } | ||
|
|
||
| /// A `Sequence` of `Date`s, calculated by iterative addition of `DateComponents`. | ||
| struct DatesByAdding : Sendable, Sequence { |
There was a problem hiding this comment.
All of these types appear to be missing visibility annotations.
There was a problem hiding this comment.
These are internal types now (the public API is just some (Sequence<Date> & Sendable).
There was a problem hiding this comment.
Ah of course; I glossed over the type-erased return types. Thanks :)
parkera
force-pushed
the
parkera/calendar_sequence
branch
from
a30c665 to
aafc8a8
Compare
| guard let next else { | ||
| // We have finished | ||
| previousResult = nil | ||
| finished = false | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Shouldn't this be finished = true, since "We have finished"?
parkera
force-pushed
the
parkera/calendar_sequence
branch
from
ca9ff44 to
809920b
Compare
|
@swift-ci test |
parkera
force-pushed
the
parkera/calendar_sequence
branch
from
809920b to
41fa6cd
Compare
|
@swift-ci test |
parkera
force-pushed
the
parkera/calendar_sequence
branch
from
41fa6cd to
4044746
Compare
|
@swift-ci test |
| @@ -1778,7 +1800,13 @@ internal final class _CalendarICU: _CalendarProtocol, @unchecked Sendable { | |||
| } while ucal_get(ucalendar, UCAL_ERA, &status) < targetEra | |||
| } | |||
|
|
|||
| if startAtUnit == .day || startAtUnit == .weekday || startAtUnit == .weekdayOrdinal { | |||
| let useDayOfMonth = if #available(FoundationPreview 0.4, *) { | |||
| startAtUnit == .day || startAtUnit == .weekday || startAtUnit == .weekdayOrdinal || startAtUnit == .dayOfYear | |||
There was a problem hiding this comment.
This looks counter intuitive?
|
@swift-ci test |
parkera
force-pushed
the
parkera/calendar_sequence
branch
from
67ec5f0 to
86e1efc
Compare
|
@swift-ci test |
1 similar comment
|
@swift-ci test |
parkera
force-pushed
the
parkera/calendar_sequence
branch
from
be42126 to
33467bb
Compare
|
@swift-ci test |
parkera
force-pushed
the
parkera/calendar_sequence
branch
from
33467bb to
7c15437
Compare
|
@swift-ci test |
See #321 for API proposal.