Skip to content

[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

Merged
merged 5 commits into from
Apr 5, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
39 changes: 39 additions & 0 deletions Guides/Compacted.md
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.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Read more about the package, and the intent behind it, in the [announcement on s
- [`reductions(_:)`, `reductions(_:_:)`](https://github.com/apple/swift-algorithms/blob/main/Guides/Reductions.md): Returns all the intermediate states of reducing the elements of a sequence or collection.
- [`split(maxSplits:omittingEmptySubsequences:whereSeparator)`, `split(separator:maxSplits:omittingEmptySubsequences)`](https://github.com/apple/swift-algorithms/blob/main/Guides/LazySplit.md): Lazy versions of the Standard Library's eager operations that split sequences and collections into subsequences separated by the specified separator element.
- [`windows(ofCount:)`](https://github.com/apple/swift-algorithms/blob/main/Guides/Windows.md): Breaks a collection into overlapping subsequences where elements are slices from the original collection.
- [`compacted()`](https://github.com/apple/swift-algorithms/blob/main/Guides/Compacted.md): Flatten the `nil`s out of a sequence of collection.

## Adding Swift Algorithms as a Dependency

Expand Down
228 changes: 228 additions & 0 deletions Sources/Algorithms/Compacted.swift
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")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
precondition(i < endIndex, "Index out of bounds")
precondition(i != endIndex, "Index out of bounds")

We try to compare indices using != wherever possible to be consistent with the stdlib, also see #65 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah ok, thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed! :)


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")

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 {
@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 {}
Copy link
Member

Choose a reason for hiding this comment

The 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-nil elements are until we iterate, we can't make CompactedCollection conform to RandomAccessCollection.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)
}
}
}
68 changes: 68 additions & 0 deletions Tests/SwiftAlgorithmsTests/CompactedTests.swift
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)
Copy link
Member

Choose a reason for hiding this comment

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

Looks like we need to call compacted() here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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()
)
}
}
Copy link
Member

Choose a reason for hiding this comment

The 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. [1, 2, 3, nil, nil, 4].compacted() == [1, nil, 2, nil, 3, 4].compacted(). You're covering the correctness of compacting already, so I don't think you need to do the comparison with compactMap { $0 } any more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adjusted :)

}
40 changes: 40 additions & 0 deletions Tests/SwiftAlgorithmsTests/TestUtilities.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,27 @@ struct SplitMix64: RandomNumberGenerator {
}
}

// An eraser helper to any hashable sequence.
struct AnyHashableSequence<Base>
where Base: Sequence, Base: Hashable {
var base: Base
}

extension AnyHashableSequence: Hashable {}
extension AnyHashableSequence: Sequence {
typealias Iterator = Base.Iterator

func makeIterator() -> Iterator {
base.makeIterator()
}
}

extension Sequence where Self: Hashable {
func eraseToAnyHashableSequence() -> AnyHashableSequence<Self> {
AnyHashableSequence(base: self)
}
}

// An eraser helper to any mutable collection
struct AnyMutableCollection<Base> where Base: MutableCollection {
var base: Base
Expand Down Expand Up @@ -163,6 +184,25 @@ func XCTAssertEqualCollections<C1: Collection, C2: Collection>(
}
}

func hash<T: Hashable>(_ value: T) -> Int {
var hasher = Hasher()
value.hash(into: &hasher)
return hasher.finalize()
}

/// Asserts two hashable value produce the same hash value.
func XCTAssertEqualHashValue<T: Hashable, U: Hashable>(
_ expression1: @autoclosure () throws -> T,
_ expression2: @autoclosure () throws -> U,
_ message: @autoclosure () -> String = "",
file: StaticString = #file, line: UInt = #line
) {
XCTAssertEqual(
hash(try expression1()), hash(try expression2()),
message(), file: file, line: line
)
}

/// Tests that all index traversal methods behave as expected.
///
/// Verifies the correctness of the implementations of `startIndex`, `endIndex`,
Expand Down