swift-algorithms icon indicating copy to clipboard operation
swift-algorithms copied to clipboard

Partition: Make mutating functions return `@discardableResult`

Open mdznr opened this issue 4 years ago • 3 comments

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

  • [x] If possible, I've reproduced the issue using the main branch of this package
  • [x] I've searched for existing GitHub issues

  1. This implementation function can be seen on page 218 of the presentation slides PDF.

mdznr avatar Jan 21 '21 21:01 mdznr

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.

kylemacomber avatar Jan 22 '21 01:01 kylemacomber

@discardableResult #67

kocsma avatar Jul 31 '21 15:07 kocsma

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

Saklad5 avatar Sep 28 '21 15:09 Saklad5