Adjacent pairs wrapping

[mdznr]

Description

Adds an optional wrapping parameter to adjacentPairs algorithm that allows you to include a pair of the last and first element.

Detailed Design

extension Sequence {
  /// …
  ///
  /// The following example uses `adjacentPairs(wrapping: true)` to iterate over
  /// adjacent pairs of integers, including the last and first:
  ///
  ///     for pair in (1...).prefix(5).adjacentPairs(wrapping: true) {
  ///         print(pair)
  ///     }
  ///     // Prints "(1, 2)"
  ///     // Prints "(2, 3)"
  ///     // Prints "(3, 4)"
  ///     // Prints "(4, 5)"
  ///     // Prints "(5, 1)"
  ///
  /// - Parameter wrapping: If `true`, include the pair of the last and first
  /// elements of the sequence as the last elements of the returned collection.
  /// Defaults to `false`.
  @inlinable
  public func adjacentPairs(wrapping: Bool = false) -> AdjacentPairsSequence<Self> {
    AdjacentPairsSequence(base: self, wrapping: wrapping)
  }
}

extension Collection {
  /// …
  ///
  /// - Parameter wrapping: If `true`, include the pair of the last and first
  /// elements of the collection as the last elements of the returned
  /// collection. Defaults to `false`.
  @inlinable
  public func adjacentPairs(wrapping: Bool = false) -> AdjacentPairsCollection<Self> {
    AdjacentPairsCollection(base: self, wrapping: wrapping)
  }
}

Alternatives Considered

• A new Sequence and Collection algorithm. This would have resulted in a lot of duplicate code from AdjacentPairs that would be better shared between the implementations.

Documentation Plan

I’ll waiting to hear feedback on whether or not this is desirable and on the direction before diving into the documentation too much.

• Improve inline documentation
• Update Guides/AdjacentPairs.md
• README.md

Test Plan

For each of the existing adjacentPairs unit tests, I’ve added one that tests the wrapping behavior as well.

Source Impact

Existing calls to adjacentPairs() continues to work as-is since the wrapping parameter has a default value of false.

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

[xwu]

wrapping suggests to me an infinite sequence (which I'd imagine has its own uses); is there potentially another way of describing this specific option?

[mdznr]

wrapping suggests to me an infinite sequence (which I'd imagine has its own uses); is there potentially another way of describing this specific option?

This is something that I struggled with. I was hoping to get some feedback on the naming. I agree that wrapping could be misinterpreted, but haven’t thought of anything better yet.

Here are some other options to get the discussion going:

• includingEnds
• includingAdjoinedEnds
• includingLastAndFirstPair
• adjoiningLastAndFirst
• adjoiningEnds
• Something using clasp (as used with jewelry to join together a series of links into a loop)

[kylemacomber]

If we added this, I presume we would want to add the same functionality to windows(ofCount:) and there'd be count - 1 wrapping entries:

let x = [1, 2, 3, 4, 5]
for window in x.windows(ofCount: 3, wrapping: true) {
  print(window)
}
// Prints [1, 2, 3]
// Prints [2, 3, 4]
// Prints [3, 4, 5]
// Prints [4, 5, 1]
// Prints [5, 1, 2]

[mdznr]

If we added this, I presume we would want to add the same functionality to windows(ofCount:) and there'd be count - 1 wrapping entries

I think you meant count (one array element in the original collection).

Due to the similarities between windows(ofCount: 2) and adjacentPairs(), it would seem we’d want to keep them in sync. However, I don’t see as strong of a use case for wrapping with a count other than 2. Perhaps that’s just because my original use case was only to help link up a circular linked list.

Perhaps instead of adding a wrapped parameter to the adjacentPairs() function, we could create a different function for the purposes of the public API (even if it shared the implementation with AdjacentPairsSequence and AdjacentPairsCollection).

[kylemacomber]

I appreciate that this was motivated by a concrete real world use case, however after thinking about this more, I'm not sure adding a wrapping flag (regardless of its name) pulls its weight.

For example, I think this behavior can be relatively trivially composed out of chain(x, x.prefix(1)).adjacentPairs():

let x = (1...).prefix(5)
for pair in chain(x, x.prefix(1)).adjacentPairs() {
    print(pair)
}
// Prints "(1, 2)"
// Prints "(2, 3)"
// Prints "(3, 4)"
// Prints "(4, 5)"
// Prints "(5, 1)"

I vote that we close this for now and revisit if additional concrete use cases arise.

[natecook1000]

Agreed, we can close this one for now. Thanks for testing this out, @mdznr — happy to discuss this more on the forums!