From 7be7d17422d51bc1300388285f7ee9fe3502debe Mon Sep 17 00:00:00 2001 From: Nate Cook Date: Tue, 4 May 2021 23:36:41 -0500 Subject: [PATCH 1/3] Update AdjacentPairs implementations and tests --- Sources/Algorithms/AdjacentPairs.swift | 220 ++++++++++-------- .../AdjacentPairsTests.swift | 97 +++----- 2 files changed, 165 insertions(+), 152 deletions(-) diff --git a/Sources/Algorithms/AdjacentPairs.swift b/Sources/Algorithms/AdjacentPairs.swift index ff4901c6..9c8ad856 100644 --- a/Sources/Algorithms/AdjacentPairs.swift +++ b/Sources/Algorithms/AdjacentPairs.swift @@ -18,13 +18,13 @@ extension Sequence { /// The following example uses the `adjacentPairs()` method to iterate over /// adjacent pairs of integers: /// - /// for pair in (1...5).adjacentPairs() { - /// print(pair) - /// } - /// // Prints "(1, 2)" - /// // Prints "(2, 3)" - /// // Prints "(3, 4)" - /// // Prints "(4, 5)" + /// for pair in (1...).prefix(5).adjacentPairs() { + /// print(pair) + /// } + /// // Prints "(1, 2)" + /// // Prints "(2, 3)" + /// // Prints "(3, 4)" + /// // Prints "(4, 5)" @inlinable public func adjacentPairs() -> AdjacentPairsSequence { AdjacentPairsSequence(base: self) @@ -32,21 +32,21 @@ extension Sequence { } extension Collection { - /// A collection of adjacent pairs of elements built from an underlying collection. + /// A collection of adjacent pairs of elements built from an underlying + /// collection. /// - /// In an `AdjacentPairsCollection`, the elements of the *i*th pair are the *i*th - /// and *(i+1)*th elements of the underlying sequence. The following example - /// uses the `adjacentPairs()` method to iterate over adjacent pairs of - /// integers: - /// ``` - /// for pair in (1...5).adjacentPairs() { - /// print(pair) - /// } - /// // Prints "(1, 2)" - /// // Prints "(2, 3)" - /// // Prints "(3, 4)" - /// // Prints "(4, 5)" - /// ``` + /// In an `AdjacentPairsCollection`, the elements of the *i*th pair are the + /// *i*th and *(i+1)*th elements of the underlying sequence. The following + /// example uses the `adjacentPairs()` method to iterate over adjacent pairs + /// of integers: + /// + /// for pair in (1...5).adjacentPairs() { + /// print(pair) + /// } + /// // Prints "(1, 2)" + /// // Prints "(2, 3)" + /// // Prints "(3, 4)" + /// // Prints "(4, 5)" @inlinable public func adjacentPairs() -> AdjacentPairsCollection { AdjacentPairsCollection(base: self) @@ -55,19 +55,8 @@ extension Collection { /// A sequence of adjacent pairs of elements built from an underlying sequence. /// -/// In an `AdjacentPairsSequence`, the elements of the *i*th pair are the *i*th -/// and *(i+1)*th elements of the underlying sequence. The following example -/// uses the `adjacentPairs()` method to iterate over adjacent pairs of -/// integers: -/// ``` -/// for pair in (1...5).adjacentPairs() { -/// print(pair) -/// } -/// // Prints "(1, 2)" -/// // Prints "(2, 3)" -/// // Prints "(3, 4)" -/// // Prints "(4, 5)" -/// ``` +/// Use the `adjacentPairs()` method on a sequence to create an +/// `AdjacentPairsSequence` instance. public struct AdjacentPairsSequence { @usableFromInline internal let base: Base @@ -124,21 +113,11 @@ extension AdjacentPairsSequence: Sequence { } } -/// A collection of adjacent pairs of elements built from an underlying collection. +/// A collection of adjacent pairs of elements built from an underlying +/// collection. /// -/// In an `AdjacentPairsCollection`, the elements of the *i*th pair are the *i*th -/// and *(i+1)*th elements of the underlying sequence. The following example -/// uses the `adjacentPairs()` method to iterate over adjacent pairs of -/// integers: -/// ``` -/// for pair in (1...5).adjacentPairs() { -/// print(pair) -/// } -/// // Prints "(1, 2)" -/// // Prints "(2, 3)" -/// // Prints "(3, 4)" -/// // Prints "(4, 5)" -/// ``` +/// Use the `adjacentPairs()` method on a collection to create an +/// `AdjacentPairsCollection` instance. public struct AdjacentPairsCollection { @usableFromInline internal let base: Base @@ -148,13 +127,25 @@ public struct AdjacentPairsCollection { @inlinable internal init(base: Base) { self.base = base + + // Lazily build the end index, since we can't use the instance + // property pre-initialization + var endIndex: Index { + Index(first: base.endIndex, second: base.endIndex) + } - // Precompute `startIndex` to ensure O(1) behavior, - // avoiding indexing past `endIndex` - let start = base.startIndex - let end = base.endIndex - let second = start == end ? start : base.index(after: start) - self.startIndex = Index(first: start, second: second) + // Precompute `startIndex` to ensure O(1) behavior. + guard !base.isEmpty else { + self.startIndex = endIndex + return + } + + // If there's only one element (i.e. the second index of base == endIndex) + // then this collection should be empty. + let secondIndex = base.index(after: base.startIndex) + self.startIndex = secondIndex == base.endIndex + ? endIndex + : Index(first: base.startIndex, second: secondIndex) } } @@ -181,9 +172,14 @@ extension AdjacentPairsCollection { self.second = second } + @inlinable + public static func ==(lhs: Index, rhs: Index) -> Bool { + lhs.first == rhs.first + } + @inlinable public static func < (lhs: Index, rhs: Index) -> Bool { - (lhs.first, lhs.second) < (rhs.first, rhs.second) + lhs.first < rhs.first } } } @@ -191,12 +187,7 @@ extension AdjacentPairsCollection { extension AdjacentPairsCollection: Collection { @inlinable public var endIndex: Index { - switch base.endIndex { - case startIndex.first, startIndex.second: - return startIndex - case let end: - return Index(first: end, second: end) - } + Index(first: base.endIndex, second: base.endIndex) } @inlinable @@ -206,6 +197,7 @@ extension AdjacentPairsCollection: Collection { @inlinable public func index(after i: Index) -> Index { + precondition(i != endIndex, "Can't advance beyond endIndex") let next = base.index(after: i.second) return next == base.endIndex ? endIndex @@ -214,38 +206,74 @@ extension AdjacentPairsCollection: Collection { @inlinable public func index(_ i: Index, offsetBy distance: Int) -> Index { - if distance == 0 { - return i - } else if distance > 0 { - let firstOffsetIndex = base.index(i.first, offsetBy: distance) - let secondOffsetIndex = base.index(after: firstOffsetIndex) - return secondOffsetIndex == base.endIndex - ? endIndex - : Index(first: firstOffsetIndex, second: secondOffsetIndex) + guard distance != 0 else { return i } + + guard let result = distance > 0 + ? offsetForward(i, by: distance, limitedBy: endIndex) + : offsetBackward(i, by: -distance, limitedBy: startIndex) + else { fatalError("Index out of bounds") } + return result + } + + @inlinable + public func index( + _ i: Index, offsetBy distance: Int, limitedBy limit: Index + ) -> Index? { + guard distance != 0 else { return i } + guard limit != i else { return nil } + + if distance > 0 { + let limit = limit > i ? limit : endIndex + return offsetForward(i, by: distance, limitedBy: limit) } else { - return i == endIndex - ? Index(first: base.index(i.first, offsetBy: distance - 1), - second: base.index(i.first, offsetBy: distance)) - : Index(first: base.index(i.first, offsetBy: distance), - second: i.first) + let limit = limit < i ? limit : startIndex + return offsetBackward(i, by: -distance, limitedBy: limit) } } + + @inlinable + internal func offsetForward( + _ i: Index, by distance: Int, limitedBy limit: Index + ) -> Index? { + assert(distance > 0) + assert(limit > i) + + guard let newFirst = base.index(i.second, offsetBy: distance - 1, limitedBy: limit.first), + newFirst != base.endIndex + else { return nil } + let newSecond = base.index(after: newFirst) + + precondition(newSecond <= base.endIndex, "Can't advance beyond endIndex") + return newSecond == base.endIndex + ? endIndex + : Index(first: newFirst, second: newSecond) + } + + @inlinable + internal func offsetBackward( + _ i: Index, by distance: Int, limitedBy limit: Index + ) -> Index? { + assert(distance > 0) + assert(limit < i) + + let offset = i == endIndex ? 0 : 1 + guard let newSecond = base.index( + i.first, + offsetBy: -(distance - offset), + limitedBy: limit.second) + else { return nil } + let newFirst = base.index(newSecond, offsetBy: -1) + precondition(newSecond >= base.startIndex, "Can't move before startIndex") + return Index(first: newFirst, second: newSecond) + } @inlinable public func distance(from start: Index, to end: Index) -> Int { - let offset: Int - switch (start.first, end.first) { - case (base.endIndex, base.endIndex): - return 0 - case (base.endIndex, _): - offset = +1 - case (_, base.endIndex): - offset = -1 - default: - offset = 0 - } - - return base.distance(from: start.first, to: end.first) + offset + // While there's a 2-step gap between the `first` base index values in + // `endIndex` and the penultimate index of this collection, the `second` + // base index values are consistently one step apart throughout the + // entire collection. + base.distance(from: start.second, to: end.second) } @inlinable @@ -259,13 +287,21 @@ extension AdjacentPairsCollection: BidirectionalCollection { @inlinable public func index(before i: Index) -> Index { - i == endIndex - ? Index(first: base.index(i.first, offsetBy: -2), - second: base.index(before: i.first)) - : Index(first: base.index(before: i.first), - second: i.first) + precondition(i != startIndex, "Can't offset before startIndex") + let second = i == endIndex + ? base.index(before: base.endIndex) + : i.first + let first = base.index(before: second) + return Index(first: first, second: second) } } extension AdjacentPairsCollection: RandomAccessCollection where Base: RandomAccessCollection {} + +extension AdjacentPairsCollection.Index: Hashable where Base.Index: Hashable { + @inlinable + public func hash(into hasher: inout Hasher) { + hasher.combine(first) + } +} diff --git a/Tests/SwiftAlgorithmsTests/AdjacentPairsTests.swift b/Tests/SwiftAlgorithmsTests/AdjacentPairsTests.swift index 3226027d..f7eab740 100644 --- a/Tests/SwiftAlgorithmsTests/AdjacentPairsTests.swift +++ b/Tests/SwiftAlgorithmsTests/AdjacentPairsTests.swift @@ -13,86 +13,63 @@ import XCTest import Algorithms final class AdjacentPairsTests: XCTestCase { + func testEmptySequence() { + let pairs = (0...).prefix(0).adjacentPairs() + XCTAssertEqualSequences(pairs, [], by: ==) + } + + func testOneElementSequence() { + let pairs = (0...).prefix(1).adjacentPairs() + XCTAssertEqualSequences(pairs, [], by: ==) + } + + func testTwoElementSequence() { + let pairs = (0...).prefix(2).adjacentPairs() + XCTAssertEqualSequences(pairs, [(0, 1)], by: ==) + } + + func testThreeElementSequence() { + let pairs = (0...).prefix(3).adjacentPairs() + XCTAssertEqualSequences(pairs, [(0, 1), (1, 2)], by: ==) + } + + func testManySequences() { + for n in 4...100 { + let pairs = (0...).prefix(n).adjacentPairs() + XCTAssertEqualSequences(pairs, zip(0..., 1...).prefix(n - 1), by: ==) + } + } + func testZeroElements() { let pairs = (0..<0).adjacentPairs() XCTAssertEqual(pairs.startIndex, pairs.endIndex) - XCTAssert(Array(pairs) == []) + XCTAssertEqualSequences(pairs, [], by: ==) } func testOneElement() { let pairs = (0..<1).adjacentPairs() XCTAssertEqual(pairs.startIndex, pairs.endIndex) - XCTAssert(Array(pairs) == []) + XCTAssertEqualSequences(pairs, [], by: ==) } func testTwoElements() { let pairs = (0..<2).adjacentPairs() - XCTAssert(Array(pairs) == [(0, 1)]) + XCTAssertEqualSequences(pairs, [(0, 1)], by: ==) } func testThreeElements() { let pairs = (0..<3).adjacentPairs() - XCTAssert(Array(pairs) == [(0, 1), (1, 2)]) - } - - func testFourElements() { - let pairs = (0..<4).adjacentPairs() - XCTAssert(Array(pairs) == [(0, 1), (1, 2), (2, 3)]) - } - - func testForwardIndexing() { - let pairs = (1...5).adjacentPairs() - let expected = [(1, 2), (2, 3), (3, 4), (4, 5)] - var index = pairs.startIndex - for iteration in expected.indices { - XCTAssert(pairs[index] == expected[iteration]) - pairs.formIndex(after: &index) - } - XCTAssertEqual(index, pairs.endIndex) + XCTAssertEqualSequences(pairs, [(0, 1), (1, 2)], by: ==) } - func testBackwardIndexing() { - let pairs = (1...5).adjacentPairs() - let expected = [(4, 5), (3, 4), (2, 3), (1, 2)] - var index = pairs.endIndex - for iteration in expected.indices { - pairs.formIndex(before: &index) - XCTAssert(pairs[index] == expected[iteration]) + func testManyElements() { + for n in 4...100 { + let pairs = (0.. (lhs: Self, rhs: Self) -> Bool where Element == (L, R) { - lhs.count == rhs.count && zip(lhs, rhs).allSatisfy(==) + func testIndexTraversals() { + validateIndexTraversals((1...5).adjacentPairs()) } } From f624ba9fdc3d708cccec632f1d12ebe02ac568a5 Mon Sep 17 00:00:00 2001 From: Nate Cook Date: Tue, 4 May 2021 23:53:09 -0500 Subject: [PATCH 2/3] Add conditional lazy conformances --- Sources/Algorithms/AdjacentPairs.swift | 8 ++++++++ Tests/SwiftAlgorithmsTests/AdjacentPairsTests.swift | 5 +++++ 2 files changed, 13 insertions(+) diff --git a/Sources/Algorithms/AdjacentPairs.swift b/Sources/Algorithms/AdjacentPairs.swift index 9c8ad856..ae7443c3 100644 --- a/Sources/Algorithms/AdjacentPairs.swift +++ b/Sources/Algorithms/AdjacentPairs.swift @@ -69,6 +69,7 @@ public struct AdjacentPairsSequence { } extension AdjacentPairsSequence { + /// The iterator for an `AdjacentPairsSequence` or `AdjacentPairsCollection`. public struct Iterator { @usableFromInline internal var base: Base.Iterator @@ -113,6 +114,9 @@ extension AdjacentPairsSequence: Sequence { } } +extension AdjacentPairsSequence: LazySequenceProtocol + where Base: LazySequenceProtocol {} + /// A collection of adjacent pairs of elements built from an underlying /// collection. /// @@ -159,6 +163,7 @@ extension AdjacentPairsCollection { } extension AdjacentPairsCollection { + /// A position in an `AdjacentPairsCollection`. public struct Index: Comparable { @usableFromInline internal var first: Base.Index @@ -299,6 +304,9 @@ extension AdjacentPairsCollection: BidirectionalCollection extension AdjacentPairsCollection: RandomAccessCollection where Base: RandomAccessCollection {} +extension AdjacentPairsCollection: LazySequenceProtocol, LazyCollectionProtocol + where Base: LazyCollectionProtocol {} + extension AdjacentPairsCollection.Index: Hashable where Base.Index: Hashable { @inlinable public func hash(into hasher: inout Hasher) { diff --git a/Tests/SwiftAlgorithmsTests/AdjacentPairsTests.swift b/Tests/SwiftAlgorithmsTests/AdjacentPairsTests.swift index f7eab740..e987b4ac 100644 --- a/Tests/SwiftAlgorithmsTests/AdjacentPairsTests.swift +++ b/Tests/SwiftAlgorithmsTests/AdjacentPairsTests.swift @@ -72,4 +72,9 @@ final class AdjacentPairsTests: XCTestCase { func testIndexTraversals() { validateIndexTraversals((1...5).adjacentPairs()) } + + func testLaziness() { + XCTAssertLazySequence((0...).lazy.adjacentPairs()) + XCTAssertLazyCollection((0..<100).lazy.adjacentPairs()) + } } From 9b173fa204cfac07143b16c3a2fb6483d3eca305 Mon Sep 17 00:00:00 2001 From: Nate Cook Date: Wed, 5 May 2021 12:06:43 -0500 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Tim Vermeulen --- Sources/Algorithms/AdjacentPairs.swift | 2 +- Tests/SwiftAlgorithmsTests/AdjacentPairsTests.swift | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Sources/Algorithms/AdjacentPairs.swift b/Sources/Algorithms/AdjacentPairs.swift index ae7443c3..002a07a6 100644 --- a/Sources/Algorithms/AdjacentPairs.swift +++ b/Sources/Algorithms/AdjacentPairs.swift @@ -268,7 +268,7 @@ extension AdjacentPairsCollection: Collection { limitedBy: limit.second) else { return nil } let newFirst = base.index(newSecond, offsetBy: -1) - precondition(newSecond >= base.startIndex, "Can't move before startIndex") + precondition(newFirst >= base.startIndex, "Can't move before startIndex") return Index(first: newFirst, second: newSecond) } diff --git a/Tests/SwiftAlgorithmsTests/AdjacentPairsTests.swift b/Tests/SwiftAlgorithmsTests/AdjacentPairsTests.swift index e987b4ac..98396aa4 100644 --- a/Tests/SwiftAlgorithmsTests/AdjacentPairsTests.swift +++ b/Tests/SwiftAlgorithmsTests/AdjacentPairsTests.swift @@ -70,7 +70,11 @@ final class AdjacentPairsTests: XCTestCase { } func testIndexTraversals() { - validateIndexTraversals((1...5).adjacentPairs()) + validateIndexTraversals( + (0..<0).adjacentPairs(), + (0..<1).adjacentPairs(), + (0..<2).adjacentPairs(), + (0..<5).adjacentPairs()) } func testLaziness() {