-
Notifications
You must be signed in to change notification settings - Fork 448
[Compacted] Adding compacted()
method to remove all nils in a sequence or collection
#112
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
Changes from 3 commits
d0d9ed5
05491db
abe4890
a42bc53
d64dd16
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
# Compacted | ||
|
||
[[Source](https://github.com/apple/swift-algorithms/blob/main/Sources/Algorithms/Compacted.swift) | | ||
[Tests](https://github.com/apple/swift-algorithms/blob/main/Tests/SwiftAlgorithmsTests/CompactedTests.swift)] | ||
|
||
Convenience method that flatten the `nil`s out of a sequence or collection. | ||
That behaves exactly one of the most common uses of `compactMap` which is `collection.lazy.compactMap { $0 }` | ||
which is only remove `nil`s without transforming the elements. | ||
|
||
```swift | ||
let array: [Int?] = [10, nil, 30, nil, 2, 3, nil, 5] | ||
let withNoNils = array.compacted() | ||
// Array(withNoNils) == [10, 30, 2, 3, 5] | ||
|
||
``` | ||
|
||
The most convenient part of `compacted()` is that we avoid the usage of a closure. | ||
|
||
## Detailed Design | ||
|
||
The `compacted()` methods has two overloads: | ||
|
||
```swift | ||
extension Sequence { | ||
public func compacted<Unwrapped>() -> CompactedSequence<Self, Unwrapped> { ... } | ||
} | ||
|
||
extension Collection { | ||
public func compacted<Unwrapped>() -> CompactedCollection<Self, Unwrapped> { ... } | ||
} | ||
``` | ||
|
||
One is a more general `CompactedSequence` for any `Sequence` base. And the other a more specialized `CompactedCollection` | ||
where base is a `Collection` and with conditional conformance to `BidirectionalCollection`, `RandomAccessCollection`, | ||
`LazyCollectionProtocol`, `Equatable` and `Hashable` when base collection conforms to them. | ||
|
||
### Naming | ||
|
||
The naming method name `compacted()` matches the current method `compactMap` that one of the most common usages `compactMap { $0 }` is abstracted by it. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,228 @@ | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// This source file is part of the Swift Algorithms open source project | ||
// | ||
// Copyright (c) 2021 Apple Inc. and the Swift project authors | ||
// Licensed under Apache License v2.0 with Runtime Library Exception | ||
// | ||
// See https://swift.org/LICENSE.txt for license information | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
/// A `Sequence` that iterates over every non-nil element from the original `Sequence`. | ||
public struct CompactedSequence<Base: Sequence, Element>: Sequence | ||
where Base.Element == Element? { | ||
|
||
@usableFromInline | ||
let base: Base | ||
|
||
@inlinable | ||
init(base: Base) { | ||
self.base = base | ||
} | ||
|
||
public struct Iterator: IteratorProtocol { | ||
@usableFromInline | ||
var base: Base.Iterator | ||
|
||
@inlinable | ||
init(base: Base.Iterator) { | ||
self.base = base | ||
} | ||
|
||
@inlinable | ||
public mutating func next() -> Element? { | ||
while let wrapped = base.next() { | ||
guard let some = wrapped else { continue } | ||
return some | ||
} | ||
return nil | ||
} | ||
} | ||
|
||
@inlinable | ||
public func makeIterator() -> Iterator { | ||
return Iterator(base: base.makeIterator()) | ||
} | ||
} | ||
|
||
extension Sequence { | ||
/// Returns a new `Sequence` that iterates over every non-nil element | ||
/// from the original `Sequence`. | ||
/// It produces the same result as `c.compactMap { $0 }`. | ||
/// | ||
/// let c = [1, nil, 2, 3, nil] | ||
/// for num in c.compacted() { | ||
/// print(num) | ||
/// } | ||
/// // 1 | ||
/// // 2 | ||
/// // 3 | ||
/// | ||
/// - Returns: A `Sequence` where the element is the unwrapped original | ||
/// element and iterates over every non-nil element from the original | ||
/// `Sequence`. | ||
/// | ||
/// Complexity: O(1) | ||
@inlinable | ||
public func compacted<Unwrapped>() -> CompactedSequence<Self, Unwrapped> | ||
where Element == Unwrapped? { | ||
CompactedSequence(base: self) | ||
} | ||
} | ||
|
||
/// A `Collection` that iterates over every non-nil element from the original `Collection`. | ||
public struct CompactedCollection<Base: Collection, Element>: Collection | ||
where Base.Element == Element? { | ||
|
||
@usableFromInline | ||
let base: Base | ||
|
||
@inlinable | ||
init(base: Base) { | ||
self.base = base | ||
let idx = base.firstIndex(where: { $0 != nil }) ?? base.endIndex | ||
self.startIndex = Index(base: idx) | ||
} | ||
|
||
public struct Index { | ||
@usableFromInline | ||
let base: Base.Index | ||
|
||
@inlinable | ||
init(base: Base.Index) { | ||
self.base = base | ||
} | ||
} | ||
|
||
public var startIndex: Index | ||
|
||
@inlinable | ||
public var endIndex: Index { Index(base: base.endIndex) } | ||
|
||
@inlinable | ||
public subscript(position: Index) -> Element { | ||
base[position.base]! | ||
} | ||
|
||
@inlinable | ||
public func index(after i: Index) -> Index { | ||
precondition(i < endIndex, "Index out of bounds") | ||
|
||
let baseIdx = base.index(after: i.base) | ||
guard let idx = base[baseIdx...].firstIndex(where: { $0 != nil }) | ||
else { return endIndex } | ||
return Index(base: idx) | ||
} | ||
} | ||
|
||
extension CompactedCollection: BidirectionalCollection | ||
where Base: BidirectionalCollection { | ||
|
||
@inlinable | ||
public func index(before i: Index) -> Index { | ||
precondition(i > startIndex, "Index out of bounds") | ||
LucianoPAlmeida marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
guard let idx = | ||
base[startIndex.base..<i.base] | ||
.lastIndex(where: { $0 != nil }) | ||
else { fatalError("Index out of bounds") } | ||
return Index(base: idx) | ||
} | ||
} | ||
|
||
extension CompactedCollection.Index: Comparable { | ||
LucianoPAlmeida marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@inlinable | ||
public static func < (lhs: CompactedCollection.Index, | ||
rhs: CompactedCollection.Index) -> Bool { | ||
lhs.base < rhs.base | ||
} | ||
} | ||
|
||
extension CompactedCollection.Index: Hashable | ||
where Base.Index: Hashable {} | ||
|
||
extension Collection { | ||
/// Returns a new `Collection` that iterates over every non-nil element | ||
/// from the original `Collection`. | ||
/// It produces the same result as `c.compactMap { $0 }`. | ||
/// | ||
/// let c = [1, nil, 2, 3, nil] | ||
/// for num in c.compacted() { | ||
/// print(num) | ||
/// } | ||
/// // 1 | ||
/// // 2 | ||
/// // 3 | ||
/// | ||
/// - Returns: A `Collection` where the element is the unwrapped original | ||
/// element and iterates over every non-nil element from the original | ||
/// `Collection`. | ||
/// | ||
/// Complexity: O(*n*) where *n* is the number of elements in the | ||
/// original `Collection`. | ||
@inlinable | ||
public func compacted<Unwrapped>() -> CompactedCollection<Self, Unwrapped> | ||
where Element == Unwrapped? { | ||
CompactedCollection(base: self) | ||
} | ||
} | ||
|
||
//===----------------------------------------------------------------------===// | ||
// Protocol Conformances | ||
//===----------------------------------------------------------------------===// | ||
|
||
extension CompactedSequence: LazySequenceProtocol | ||
where Base: LazySequenceProtocol {} | ||
|
||
extension CompactedCollection: RandomAccessCollection | ||
where Base: RandomAccessCollection {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I missed this — because we don't know how far apart the non- There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense :) |
||
extension CompactedCollection: LazySequenceProtocol | ||
where Base: LazySequenceProtocol {} | ||
extension CompactedCollection: LazyCollectionProtocol | ||
where Base: LazyCollectionProtocol {} | ||
|
||
|
||
// Hashable and Equatable conformance are based on each non-nil | ||
// element on base collection. | ||
extension CompactedSequence: Equatable | ||
where Base.Element: Equatable { | ||
|
||
@inlinable | ||
public static func ==(lhs: CompactedSequence, | ||
rhs: CompactedSequence) -> Bool { | ||
lhs.elementsEqual(rhs) | ||
} | ||
} | ||
|
||
extension CompactedSequence: Hashable | ||
where Element: Hashable { | ||
|
||
@inlinable | ||
public func hash(into hasher: inout Hasher) { | ||
for element in self { | ||
hasher.combine(element) | ||
} | ||
} | ||
} | ||
|
||
extension CompactedCollection: Equatable | ||
where Base.Element: Equatable { | ||
|
||
@inlinable | ||
public static func ==(lhs: CompactedCollection, | ||
rhs: CompactedCollection) -> Bool { | ||
lhs.elementsEqual(rhs) | ||
} | ||
} | ||
|
||
extension CompactedCollection: Hashable | ||
where Element: Hashable { | ||
|
||
@inlinable | ||
public func hash(into hasher: inout Hasher) { | ||
for element in self { | ||
hasher.combine(element) | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// This source file is part of the Swift Algorithms open source project | ||
// | ||
// Copyright (c) 2021 Apple Inc. and the Swift project authors | ||
// Licensed under Apache License v2.0 with Runtime Library Exception | ||
// | ||
// See https://swift.org/LICENSE.txt for license information | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
import XCTest | ||
import Algorithms | ||
|
||
final class CompactedTests: XCTestCase { | ||
|
||
let tests: [[Int?]] = | ||
[nil, nil, nil, 0, 1, 2] | ||
.uniquePermutations(ofCount: 0...) | ||
.map(Array.init) | ||
|
||
func testCompactedCompacted() { | ||
for collection in self.tests { | ||
let seq = AnySequence(collection) | ||
XCTAssertEqualSequences( | ||
seq.compactMap({ $0 }), seq.compacted()) | ||
XCTAssertEqualSequences( | ||
collection.compactMap({ $0 }), collection.compacted()) | ||
} | ||
} | ||
|
||
func testCompactedBidirectionalCollection() { | ||
for array in self.tests { | ||
XCTAssertEqualSequences(array.compactMap({ $0 }).reversed(), | ||
array.compacted().reversed()) | ||
} | ||
} | ||
|
||
func testCollectionTraversals() { | ||
for array in self.tests { | ||
validateIndexTraversals(array) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like we need to call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry for that |
||
} | ||
} | ||
|
||
func testCollectionEquatableConformances() { | ||
for array in self.tests { | ||
XCTAssertEqual( | ||
array.eraseToAnyHashableSequence().compacted(), | ||
array.compactMap({ $0 }).eraseToAnyHashableSequence().compacted() | ||
) | ||
XCTAssertEqual( | ||
array.compacted(), array.compactMap({ $0 }).compacted() | ||
) | ||
} | ||
} | ||
|
||
func testCollectionHashableConformances() { | ||
for array in self.tests { | ||
let seq = array.eraseToAnyHashableSequence() | ||
XCTAssertEqualHashValue( | ||
seq.compacted(), seq.compactMap({ $0 }).compacted() | ||
) | ||
XCTAssertEqualHashValue( | ||
array.compacted(), array.compactMap({ $0 }).compacted() | ||
) | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For these two tests it would be great to make sure that equality and hashing work for non-equal base sequences/collections that are equal when compacted. E.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adjusted :) |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We try to compare indices using
!=
wherever possible to be consistent with the stdlib, also see #65 (comment)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed! :)