apple / swift-algorithms

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

add suffix:while method #65

Closed michiboo closed 3 years ago

michiboo commented 3 years ago
Thanks for contributing to Swift Algorithms!

- [x] I've completed this task
- [ ] This task isn't completed

Add a suffix(while:) method #62

Checklist

CTMacUser commented 3 years ago

What about something like (neither compiled nor tested):

extension BidirectionalCollection {
  public func suffix(
    while predicate: (Element) throws -> Bool
  ) rethrows -> SubSequence {
    let start = startIndex
    var result = endIndex
    while result > start {
      let previous = index(before: result)
      guard try predicate(self[previous]) else { break }

      result = previous
    }
    return self[result...]
  }
}

?

mdznr commented 3 years ago

@CTMacUser I think that’s pretty much exactly what I imagined the implementation would look like.

Although, I think we might want to change

while result > start {

to

while result != start {

A lot of similar algorithms in the standard library use != instead of > or < for comparing indexes. I’ll have to do more research as to why that is. My initial guess is perhaps because implementations of custom indexes have slightly faster implementations of != than <. But either way, it would be good to have consistency here.

kylemacomber commented 3 years ago

A lot of similar algorithms in the standard library use != instead of > or < for comparing indexes. I’ll have to do more research as to why that is. My initial guess is perhaps because implementations of custom indexes have slightly faster implementations of != than <. But either way, it would be good to have consistency here.

I agree that we should use != to be consistent with the stdlib.

One of the reasons stdlib uses != is historical. Originally Collection.Index in the stdlib was only Equatable, not Comparable. From SE-0065 which added the Comparable requirement:

It's worth noting that this requirement isn't strictly necessary. Without it, though, indices would have no requirements beyond Equatable, and creation of a Range<T> would have to be allowed for any T conforming to Equatable. As a consequence, most interesting range operations, such as containment checks, would be unavailable unless T were also Comparable, and we'd be unable to provide bounds-checking in the general case.

That said, the requirement has real benefits. For example, it allows us to support distance measurement between arbitrary indices, even in collections without random access traversal. In the old model, x.distance(to: y) for these collections had the undetectable precondition that x precede y, with unpredictable consequences for violation in the general case.

natecook1000 commented 3 years ago

This is all set now — thanks for this implementation, @michiboo!