apple / swift-algorithms

Commonly used sequence and collection algorithms for Swift
Apache License 2.0
5.81k stars 428 forks source link

Partition: Make mutating functions return `@discardableResult` #67

Open mdznr opened 3 years ago

mdznr commented 3 years ago

Using the mutating partitioning functions are useful even when the returned index isn’t used.

The Embracing Algorithms (WWDC 2018) session implements a bringForward(elementsSatisfying:) function on MutableCollection.1 It uses stablePartition(by:) in its implementation, but doesn’t need its return value, resulting in a warning.

Actual behavior

extension MutableCollection {
    mutating func bringForward(elementsSatisfying predicate: (Element) -> Bool) {
        if let predecessor = indexBeforeFirst(where: predicate) {
            self[predecessor...].stablePartition(by: { !predicate($0) }) // ⚠️ Result of call to 'stablePartition(by:)' is unused
        }
    }
}

Expected behavior

extension MutableCollection {
    mutating func bringForward(elementsSatisfying predicate: (Element) -> Bool) {
        if let predecessor = indexBeforeFirst(where: predicate) {
            self[predecessor...].stablePartition(by: { !predicate($0) })
        }
    }
}

While it could be argued that bringForward(elementsSatisfying:) should return an Index as well, even that return value isn’t always needed to be used and should be marked as @discardableResult.

Checklist


  1. This implementation function can be seen on page 218 of the presentation slides PDF.
kylemacomber commented 3 years ago

It's a good question.

I can't recall, but I imagine we just followed the precedent set by partition(by:) from the standard library which does not mark its result as a @discardableResult.

While it could be argued that bringForward(elementsSatisfying:) should return an Index as well, even that return value isn’t always needed to be used and should be marked as @discardableResult.

I do think bringForward(elementsSatisfying:) should probably return an Index (one of Alexander Stepanov's API design principles is "don't throw away useful information"), but I agree that should arguably be marked as @discardableResult.

kocsma commented 2 years ago

@discardableResult #67

Saklad5 commented 2 years ago

The standard library can also be changed. If not now, then in Swift 6.