Skip to content

Add trimmingSuffix, trimmingPrefix and mutating variants #104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 25, 2021

Conversation

fedeci
Copy link
Contributor

@fedeci fedeci commented Mar 13, 2021

Description

Closes #101
trimming(while:) is split in two methods trimmingPrefix(while:) which is available to all Collections and trimmingSuffix(while:) available only to BidirectionalCollections.
This also adds a mutable variant of each trim method where SubSequence == Self.

Detailed Design

func trimmingSuffix(
  while predicate: (Element) throws -> Bool
) rethrows -> SubSequence {
  let end = try self[startIndex...].startOfSuffix(while: predicate)
  return self[startIndex..<end]
}

func trimmingPrefix(
  while predicate: (Element) throws -> Bool
) rethrows -> SubSequence {
  let start = try endOfPrefix(while: predicate)
  return self[start..<endIndex]
}

func trimming(
  while predicate: (Element) throws -> Bool
) rethrows -> SubSequence {
  return try trimmingPrefix(while: predicate).trimmingSuffix(while: predicate)
}

The same applies to mutable trims.

  • String must access mutable trims

Documentation Plan

Docs not yet updated.

Test Plan

Since the two new methods are just a refactoring of trimming all those tests also apply to the new API.

  • Mutating variants tests added.

Source Impact

It doesn't break anything.

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


extension BidirectionalCollection where Self == SubSequence {
@inlinable
public mutating func trim(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For what I understood from the issue we should also add an unconstrained version of trim, right @timvermeulen ?

Copy link
Member

@natecook1000 natecook1000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, @fedeci! 👏

I have a few notes for you below, and we also need a couple overloads for each of the mutating methods. The current ones are available for RangeReplaceableCollection types; we can also do these operations on self-slicing types (e.g. where Self == Self.SubSequence). One such type is a Range<Int> — when you write (0..<10).dropFirst(3) you end up with the range 3..<10. Unfortunately, there are also self-slicing, range-replaceable types (like Substring and ArraySlice), so we'll also need overloads that have both constraints.

@fedeci fedeci requested a review from natecook1000 April 2, 2021 11:07
@fedeci
Copy link
Contributor Author

fedeci commented Apr 2, 2021

Do Substring and ArraySlice inherit the mutating methods from BidirectionalCollection where Self: RangeReplaceableCollection, don't they?

Copy link
Member

@natecook1000 natecook1000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fedeci Looking good! A few notes below and then we can merge this addition.

@fedeci
Copy link
Contributor Author

fedeci commented Apr 13, 2021

Are still missing documents in the docs/ folder.

@natecook1000
Copy link
Member

@fedeci This looks ready to go! Would you like to add a skeleton guide and mark this as ready for review?

@fedeci
Copy link
Contributor Author

fedeci commented May 4, 2021

Sure, just give me a couple of days because I am full of exams in this period.


edit: weeks probably. Sorry :(

@natecook1000
Copy link
Member

@swift-ci Please test

@fedeci
Copy link
Contributor Author

fedeci commented May 21, 2021

Here it is :)

@fedeci fedeci marked this pull request as ready for review May 21, 2021 16:34
@fedeci fedeci force-pushed the feature/trimming-prefix-suffix branch 2 times, most recently from ee65d3b to 46b11ba Compare May 21, 2021 16:36
@fedeci fedeci force-pushed the feature/trimming-prefix-suffix branch from 46b11ba to 8a67ef3 Compare May 21, 2021 16:37
@@ -48,6 +48,25 @@ func myAlgorithm2<Input>(input: Input) where Input: BidirectionalCollection {
Swift provides the `BidirectionalCollection` protocol for marking types which support reverse traversal,
and generic types and algorithms which want to make use of that should add it to their constraints.

### >= 0.3.0
Copy link
Contributor Author

@fedeci fedeci May 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added 0.3.0 supposing that this will be released in a next minor version, but I am not sure.

@natecook1000
Copy link
Member

@swift-ci Please test

@natecook1000
Copy link
Member

🎉 :shipit:

@natecook1000 natecook1000 merged commit 0e2941e into apple:main May 25, 2021
@fedeci fedeci deleted the feature/trimming-prefix-suffix branch May 26, 2021 13:46
sakibguy added a commit to sakibguy/swift-algorithms that referenced this pull request May 27, 2021
Add `trimmingSuffix`, `trimmingPrefix` and mutating variants (apple#104)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add trimmingPrefix(while:), trimmingSuffix(while:), and mutating variants
3 participants