Add BidirectionalCollection.trimming

[karwa]

Description

Adds a '.trimming' method to BidirectionalCollection, returning a slice with matching elements removed from the start and end.

Detailed Design

extension BidirectionalCollection {

  /// Returns a `SubSequence` formed by discarding all elements at the start and end of the Collection
  /// which satisfy the given predicate.
  ///
  /// e.g. `[2, 10, 11, 15, 20, 21, 100].trimmimg(where: { $0.isMultiple(of: 2) })` == `[11, 15, 20, 21]`
  ///
  /// - parameters:
  ///    - predicate:  A closure which determines if the element should be omitted from the resulting slice.
  ///
  @inlinable
  public func trimming(where predicate: (Element) throws -> Bool) rethrows -> SubSequence
}

Documentation Plan

No existing guides need updating AFAIK. I haven't written a guide for this feature: is it necessary for every API to include this parallel documentation?

Test Plan

Tests are included.

Source Impact

Additive.

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

[karwa]

In terms of code style, I think my defaults match this project (except that I use a 120 line length).

Does this project just use the default swift-format settings?

[natecook1000]

Can you use existing stdlib methods here, instead of searching for the start and end manually?

[karwa]

I can use firstIndex(where:) to find the start, but not lastIndex(where:) to find the end (since we actually want the index after the last index to match the predicate; this would mean an extra call to index(after:)).

I've tried to improve the documentation, too. It might be worth adding a String.trimmed() function which defaults to trimming whitespace.

[natecook1000]

Thanks for this, @karwa! 🎉

We do need a guide for this addition — the symbol docs describe the usage and semantics, while the guide can cover more about the intention, design, what choices were considered but not taken, and how this feature relates to other similar ones in other languages. A survey of the naming in other languages would be interesting to see — I think I've seen this called chop, chomp, and strip in addition to trim.

I've also been thinking a bit about how this relates to what we already have in the standard library. The matching existing method is drop(while:), which returns a subsequence without matching elements at the beginning. That method suffers from a couple different naming issues. First, when used with a trailing predicate, it's really unclear at the use site (e.g. array.drop { $0.isEven } looks like it would drop all the even numbers). Second, there isn't really an affordance for adding an analogous method that operates from the other end, dropping trailing elements that match a predicate.

All that makes me think that "trim" may be a better root verb for all three operations — maybe trimmingPrefix(where:), trimming(where:), and trimmingSuffix(where:)? What do you think?

[karwa]

@natecook1000 I've added a guide document. It may be a little verbose, I don't know 😅. That's just my style - it's how I write, but I did my best.

I agree with trim being a better root verb -- or trimmed as an adjective, which reads like "this is the collection, trimmed of these elements", with the verb form perhaps replacing/expanding on the mutating removeFirst/Last(Int) methods. I've mentioned the various methods in the standard library in the guide document. I tried not to get too bogged-down in that discussion, otherwise it'd turn more in to a swift-evolution pitch than a useful document for people using this library.

[karwa]

@natecook1000 is there anything remaining for me to do here?

[natecook1000]

Looking good! Just a couple notes about the documentation.

[natecook1000]

Agreed, BidirectionalCollection is the right place for this.

[natecook1000]

Suggested change

/// [1]: https://en.wikipedia.org/wiki/Trimming_(computer_programming)

We should mention that if the entire collection is trimmed, the result is an empty subsequence at the end of the collection.

[karwa]

Personally I'd prefer to leave the position of the empty slice unspecified. If we decided we wanted to change this to first trim elements from the tail and then the head, the result when trimming the entire collection would be an empty subsequence at startIndex.

[xwu]

Q: can these bounds be safely unchecked too?

[karwa]

I'm not 100% sure. Since sliceEnd is formed by walking backwards from endIndex and has not yet tested equal to sliceStart, the only way this bounds check will fail is if either your collection or the index's Comparable implementation is broken.

For the empty result, we already checked that sliceStart == sliceEnd as part of the while loop condition, so it is safe to omit the bounds check. A broken Equatable implementation would never get to that part.

[xwu]

Just a teeny bit of bikeshedding:

Because trailing closure syntax causes where: to be omitted, there can be a little bit of ambiguity as to whether the closure determines what's included or what's excluded. It so happens that, in English, "trimming" can take a direct object, while "trimmed" cannot, such that [1, 2, 3].trimming { $0 == 1 } suggests that every element equal to one is being dropped a little more strongly than "trimmed" would:

"123abc", trim numerals → "abc"
"123abc", trimming numerals → "abc"
"123abc", trimmed numerals → "123" or "abc"?

[karwa]

Yeah, that sounds good. I like trimming.

[xwu]

A remark on documentation.

[karwa]

Apologies for the delay. I've addressed your comments @xwu and @natecook1000 - documentation has been improved, and I think Xiaodi makes a good case for the name trimming. Code in the tests reads quite nicely after this, e.g.:

let results = [2, 10, 11, 15, 20, 21, 100].trimming { $0.isMultiple(of: 2) }