Skip to content

Add new FiniteCycle type as the return type of Collection.cycled(times:) #106

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

Merged
merged 19 commits into from
Apr 7, 2021
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
c636b09
[test] Divide up existing Cycle tests and add FiniteCycle placeholder…
LemonSpike Mar 14, 2021
b4d0636
Implement FiniteCycle with Iterator wrapper around Product2 Iterator.
LemonSpike Mar 14, 2021
70a2c56
Change method signature of cycled(times:) to use new FiniteCycle.
LemonSpike Mar 14, 2021
3488868
Add tuple labels to Product2 for clarity.
LemonSpike Mar 14, 2021
a38d07a
Apply column 80 width consistently.
LemonSpike Mar 14, 2021
fc2cdc1
Make base and times internal to avoid future Equatable issues.
LemonSpike Mar 14, 2021
4c4e36f
Refactor times and base to single Product2 type, and refactor Iterator.
LemonSpike Mar 20, 2021
9f63577
Add Collection, BidrectionalCollection and RandomAccessCollection con…
LemonSpike Mar 20, 2021
c257737
Add tests for index methods on FiniteCycle.
LemonSpike Mar 20, 2021
d876f8b
Add new count property to cycle with a test.
LemonSpike Mar 20, 2021
037b09a
Add @inlinable annotations.
LemonSpike Mar 24, 2021
154f738
Remove Sequence conformance for FiniteCycle and add LazyCollectionPro…
LemonSpike Mar 24, 2021
952afb0
Update Cycle.md.
LemonSpike Mar 24, 2021
62abcb4
Add conditional conformances for Equatable and Hashable.
LemonSpike Mar 24, 2021
6073193
Make more @inlinable.
LemonSpike Mar 24, 2021
f65f130
Refactor typealias Index into struct, and remove Iterator.
LemonSpike Apr 5, 2021
5527e1d
Make let product internal with @usableFromInline.
LemonSpike Apr 5, 2021
66b6aff
Revert tuple labels.
LemonSpike Apr 6, 2021
e583de9
Update documentation descriptions.
LemonSpike Apr 7, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Guides/Cycle.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@ Two new methods are added to collections:
extension Collection {
func cycled() -> Cycle<Self>

func cycled(times: Int) -> FlattenSequence<Repeated<Self>>
func cycled(times: Int) -> FiniteCycle<Self>
}
```

The new `Cycle` type is a sequence only, given that the `Collection` protocol
design makes infinitely large types impossible/impractical. `Cycle` also
conforms to `LazySequenceProtocol` when the base type conforms.

Note that despite its name, the returned `FlattenSequence` will always have
`Collection` conformance, and will have `BidirectionalCollection` conformance
Note that the returned `FiniteCycle` will always have `Collection`
conformance, and will have `BidirectionalCollection` conformance
when called on a bidirectional collection.

### Complexity
Expand Down
113 changes: 111 additions & 2 deletions Sources/Algorithms/Cycle.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,115 @@ extension Cycle: Sequence {

extension Cycle: LazySequenceProtocol where Base: LazySequenceProtocol {}


/// A collection wrapper that repeats the elements of a base collection for a
/// finite number of times.
public struct FiniteCycle<Base: Collection> {
/// A Product2 instance for iterating the Base collection.
@usableFromInline
internal let product: Product2<Range<Int>, Base>

@inlinable
internal init(base: Base, times: Int) {
self.product = Product2(0..<times, base)
}
}

extension FiniteCycle: LazySequenceProtocol, LazyCollectionProtocol
where Base: LazyCollectionProtocol { }

extension FiniteCycle: Collection {

public typealias Element = Base.Element

public struct Index: Comparable {
/// The index corresponding to the Product2 index at this position.
@usableFromInline
internal let productIndex: Product2<Range<Int>, Base>.Index

@inlinable
internal init(_ productIndex: Product2<Range<Int>, Base>.Index) {
self.productIndex = productIndex
}

@inlinable
public static func == (lhs: Index, rhs: Index) -> Bool {
lhs.productIndex == rhs.productIndex
}

@inlinable
public static func < (lhs: Index, rhs: Index) -> Bool {
lhs.productIndex < rhs.productIndex
}
}

@inlinable
public var startIndex: Index {
Index(product.startIndex)
}

@inlinable
public var endIndex: Index {
Index(product.endIndex)
}

@inlinable
public subscript(_ index: Index) -> Element {
product[index.productIndex].element2
}

@inlinable
public func index(after i: Index) -> Index {
let productIndex = product.index(after: i.productIndex)
return Index(productIndex)
}

@inlinable
public func distance(from start: Index, to end: Index) -> Int {
product.distance(from: start.productIndex, to: end.productIndex)
}

@inlinable
public func index(_ i: Index, offsetBy distance: Int) -> Index {
let productIndex = product.index(i.productIndex, offsetBy: distance)
return Index(productIndex)
}

@inlinable
public func index(
_ i: Index,
offsetBy distance: Int,
limitedBy limit: Index
) -> Index? {
guard let productIndex = product.index(i.productIndex,
offsetBy: distance,
limitedBy: limit.productIndex) else {
return nil
}
return Index(productIndex)
}

@inlinable
public var count: Int {
product.count
}
}

extension FiniteCycle: BidirectionalCollection
where Base: BidirectionalCollection {
@inlinable
public func index(before i: Index) -> Index {
let productIndex = product.index(before: i.productIndex)
return Index(productIndex)
}
}

extension FiniteCycle: RandomAccessCollection
where Base: RandomAccessCollection {}

extension FiniteCycle: Equatable where Base: Equatable {}
extension FiniteCycle: Hashable where Base: Hashable {}

//===----------------------------------------------------------------------===//
// cycled()
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -109,7 +218,7 @@ extension Collection {
///
/// - Complexity: O(1)
@inlinable
public func cycled(times: Int) -> FlattenSequence<Repeated<Self>> {
repeatElement(self, count: times).joined()
public func cycled(times: Int) -> FiniteCycle<Self> {
FiniteCycle(base: self, times: times)
}
}
12 changes: 7 additions & 5 deletions Sources/Algorithms/Product.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ extension Product2: Sequence {
}

@inlinable
public mutating func next() -> (Base1.Element, Base2.Element)? {
public mutating func next() -> (element1: Base1.Element,
element2: Base2.Element)? {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's keep this tuple unlabeled — Zip2Sequence's element tuple doesn't have labels either, and generic labels such as element1 and element2 don't really add any clarity over .0 and .1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am happy to do this, but I still think that the labels improve readability for users who don't know about tuple indexing. I saw we use more descriptive names for public APIs and type names, so I think it could help users understand the API. Let me know your thoughts.

Copy link
Contributor

Choose a reason for hiding this comment

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

These labels don't improve the users' understanding, because they don't convey any useful information. It's impossible for us to give the tuple elements descriptive names because at this level of abstraction we have nothing to go on. To quote the API Design Guidelines:

Omit needless words. Every word in a name should convey salient information at the use site.

Feel free to start a discussion about this topic on the forums, as this is equally relevant to the Zip2Sequence element type. For the time being we'd like to stick with the precedent set by the standard library.

// This is the initial state, where i1.next() has never
// been called, or the final state, where i1.next() has
// already returned nil.
Expand All @@ -57,7 +58,7 @@ extension Product2: Sequence {
// Get the next element from the second sequence, if not
// at end.
if let element2 = i2.next() {
return (element1!, element2)
return (element1: element1!, element2: element2)
}

// We've reached the end of the second sequence, so:
Expand All @@ -70,7 +71,7 @@ extension Product2: Sequence {

i2 = base2.makeIterator()
if let element2 = i2.next() {
return (element1, element2)
return (element1: element1, element2: element2)
} else {
return nil
}
Expand Down Expand Up @@ -121,8 +122,9 @@ extension Product2: Collection where Base1: Collection {
}

@inlinable
public subscript(position: Index) -> (Base1.Element, Base2.Element) {
(base1[position.i1], base2[position.i2])
public subscript(position: Index) -> (element1: Base1.Element,
element2: Base2.Element) {
(element1: base1[position.i1], element2: base2[position.i2])
}

/// Forms an index from a pair of base indices, normalizing
Expand Down
56 changes: 49 additions & 7 deletions Tests/SwiftAlgorithmsTests/CycleTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,70 @@ final class CycleTests: XCTestCase {
[1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4],
cycle.prefix(20)
)
}

func testCycleClosedRangePrefix() {
let a = Array((0..<17).cycled().prefix(10_000))
XCTAssertEqual(10_000, a.count)

}

func testEmptyCycle() {
let empty = Array("".cycled())
XCTAssert(empty.isEmpty)
}


func testCycleLazy() {
XCTAssertLazySequence((1...4).lazy.cycled())
}

func testRepeated() {
let repeats = (1...4).cycled(times: 3)
XCTAssertEqualSequences(
[1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4],
repeats)

}

func testRepeatedClosedRange() {
let repeats = Array((1..<5).cycled(times: 2500))
XCTAssertEqual(10_000, repeats.count)
}

func testRepeatedEmptyCollection() {
let empty1 = Array("".cycled(times: 100))
XCTAssert(empty1.isEmpty)

}

func testRepeatedZeroTimesCycle() {
let empty2 = Array("Hello".cycled(times: 0))
XCTAssert(empty2.isEmpty)
}

func testCycleLazy() {
XCTAssertLazySequence((1...4).lazy.cycled())

func testRepeatedLazy() {
XCTAssertLazySequence((1...4).lazy.cycled(times: 3))
}

func testRepeatedIndexMethods() {
let cycle = (1..<5).cycled(times: 2)
let startIndex = cycle.startIndex
var nextIndex = cycle.index(after: startIndex)
XCTAssertEqual(cycle.distance(from: startIndex, to: nextIndex), 1)

nextIndex = cycle.index(nextIndex, offsetBy: 5)
XCTAssertEqual(cycle.distance(from: startIndex, to: nextIndex), 6)

nextIndex = cycle.index(nextIndex, offsetBy: 2, limitedBy: cycle.endIndex)!
XCTAssertEqual(cycle.distance(from: startIndex, to: nextIndex), 8)

let outOfBounds = cycle.index(nextIndex, offsetBy: 1,
limitedBy: cycle.endIndex)
XCTAssertNil(outOfBounds)

let previousIndex = cycle.index(before: nextIndex)
XCTAssertEqual(cycle.distance(from: startIndex, to: previousIndex), 7)
}

func testRepeatedCount() {
let cycle = (1..<5).cycled(times: 2)
XCTAssertEqual(cycle.count, 8)
}
}