apple / swift-algorithms

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

Add `trimmingPrefix(while:)`, `trimmingSuffix(while:)`, and mutating variants #101

Closed timvermeulen closed 3 years ago

timvermeulen commented 3 years ago

We currently provide just one way to trim a collection:

extension BidirectionalCollection {
  public func trimming(
    while predicate: (Element) throws -> Bool
  ) rethrows -> SubSequence {
    let start = try endOfPrefix(while: predicate)
    let end = try self[start...].startOfSuffix(while: predicate)
    return self[start..<end]
  }
}

We should also provide ways to trim only the start of the collection (trimmingPrefix(while:), available to all collections), or only the end (trimmingSuffix(while:)).

Furthermore, collections for which Self == SubSequence should have mutating variants using trim as the base name. As an example, Collection's popFirst() from the standard library is implemented using the same constraint.

And finally, despite String not being its own SubSequence, String should also have access to these mutating variants for the sake of convenience.

kylemacomber commented 3 years ago

I wonder if we should name these using "prefix" and "suffix" rather than "start" and "end"—trimmingPrefix(while:) and trimmingSuffix(while:)

timvermeulen commented 3 years ago

Yep, that's what I meant to say 😅

fedeci commented 3 years ago

I'd like to work on this!

natecook1000 commented 3 years ago

Could we also have the mutating methods on RangeReplaceableCollection, by replacing the whole range of self with the trimmed subsequence? I think we'd probably need tiebreakers for range-replaceable collections that are also self-slicing, but that would cover String without the type-specific overloads.

fedeci commented 3 years ago

Since trimming and trimmingSuffix are only available to BidirectionalCollection because they depend on startOfSuffix they cannot be used with RangeReplacableCollection. I came up with a solution to replace the current implementation of startOfSuffix to make it available also to Collection and not only to BidirectionalCollection.

@usableFromInline
  internal func startOfSuffix(
    while predicate: (Element) throws -> Bool
  ) rethrows -> Index {
    var index = startIndex
    var startOfSuffix: Index?

    while index != endIndex {
      if try predicate(self[index]) {
        if startOfSuffix == nil {
          startOfSuffix = index          
        }
      } else {
        startOfSuffix = nil
      }
      formIndex(after: &index)
    }
    return startOfSuffix ?? endIndex
  }

It is still quite raw and can be simplified (recursion maybe), but should work. What do you think?

timvermeulen commented 3 years ago

@fedeci Could we, instead of changing startOfSuffix, require that the collection is both RangeReplaceableCollection and BidirectionalCollection? The reason that trimming from the end is only available on bidirectional collections is that it makes the performance of the operation depend only on the size of the part being trimmed, rather than the size of the entire collection.