Add random-access methods to SlidingWindows

[timvermeulen]

index(_:offsetBy:), index(_:offsetBy:limitedBy:), and distance(from:to:) for SlidingWindows.

The base.endIndex..<base.endIndex representation of endIndex made this a bit tricky, because neither of the bounds is a logical successor of the index before it. The offsetForward/offsetBackward dance alleviates this somewhat by ensuring that the limit is always on the correct side of i and never equal to it, removing some of the edge cases.

Checklist

• I've added at least one test that validates that my change is working, if appropriate
• I've followed the code style of the rest of the project
• I've read the Contribution Guidelines
• I've updated the documentation if necessary

[ollieatkinson]

The base.endIndex..<base.endIndex representation of endIndex made this a bit tricky

Do you think it's worth considering a better implementation of endIndex in this case?

[timvermeulen]

Do you think it's worth considering a better implementation of endIndex in this case?

That's always worth considering, but I don't think there is any! If we want endIndex to fit in naturally with the other indices, its lower and upper bounds would need to be the direct successors of the bounds of the index before endIndex. But since the index before it corresponds to the last window which has an upper bound of base.endIndex, there is no successor to that (in the general case).

Of course we could give SlidingWindows.Index an internal enum representation with an endIndex case, which I'm mostly indifferent towards, but that wouldn't get rid of any edge cases. It might make it less error-prone to work with, but with the tests passing I'm pretty confident this implementation is correct as well.

Having said all that, it's totally possible to write a protocol ValidIndexCollection: Collection { ... } that lets you write things like

struct CollectionOfTwo<Element> {
  let first, second: Element
}

extension CollectionOfTwo: ValidIndexCollection {
  enum ValidIndex: Comparable {
    case first, second
  }
  
  func element(at index: ValidIndex) -> Element {
    index == .first ? first : second
  }
  
  var firstValidIndex: ValidIndex? { .first }
  var lastValidIndex: ValidIndex? { .second }
  
  func validIndex(after index: ValidIndex) -> ValidIndex {
    .second
  }
}

where ValidIndexCollection handles all the endIndex edge cases for you. This could be useful for implementing collections that don't have a natural way of representing endIndex. SlidingWindows and ClosedRange come to mind. This could be worth considering if we end up with other collections that need to deal with endIndex separately.

[ollieatkinson]

Instead of force unwrapping here (and offsetBackward) I would add an error message describing why the offset was not valid. We did something similar here

[ollieatkinson]

👏 This is excellent and a massive help when reviewing this code

[natecook1000]

Thanks, @timvermeulen! 👏