Add `ends(with:)` by amomchilov · Pull Request #224 · apple/swift-algorithms

amomchilov · 2024-02-18T23:08:59Z

This PR adds an end-counterpart to the built-in Sequence.starts(with:).

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

amomchilov · 2024-02-18T23:11:04Z

+    var possibleSuffixIterator = possibleSuffix.reversed().makeIterator()
+    for e0 in self.reversed() {
+      if let e1 = possibleSuffixIterator.next() {


Is there an efficient way to implement this without needing backwards iteration of the possibleSuffix?

The only way I can think of is basically a substring search, and checking if it ends up match against the very end.

If such an algorithm existed, it would allow us to loosen the requirement of possibleSuffix from BidirectionalCollection down to just Sequence.

If we loosen the requirement to just Collection (so that we have a known count), then we can probably iterate from the middle (starting at self.index(self.endIndex, offsetBy: possibleSuffix.count), and matching until the end. I'm not sure if this is better though, because we need to do this (potentially-non-constant-time) index math.

Right … I don't imagine that there are many cases where you have a bidirectional collection but you can't make the suffix you're searching for bidirectional.

I agree.

For completeness, I'll mention an alternative implementation I toyed with, which has different constraints:

self needs to be RandomAccessCollection instead of Bidirectional (and it does some indexing offsetting, which can be slow if that's not O(1)).

The possiblePrefix only needs to be a Collection instead of a BidirectionalCollection

extension RandomAccessCollection where Element: Equatable { @inlinable public func ends<PossibleSuffix: Collection>( with possibleSuffix: PossibleSuffix, by areEquivalent: (Element, PossibleSuffix.Element) throws -> Bool ) rethrows -> Bool { guard let startOfSuffix = index(endIndex, offsetBy: -possibleSuffix.count, limitedBy: startIndex) else { return false } return try self[startOfSuffix...].elementsEqual(possibleSuffix, by: areEquivalent) } }

Just a small addition here is that in this case possibleSuffix probably will be better if constrained to RandomAccessCollection as well, otherwise -possibleSuffix.count is not guaranteed to be O(1).

amomchilov · 2024-02-18T23:12:40Z

One of the pages of the "Guides" should link to this, but I'm not sure which.

I don't see any predicate methods like this elsewhere in the package. Perhaps we should start a new Guides/Predicates.md category?

any predicate methods like this

Can you say more about what you mean here?

I was thinking that perhaps this can be the first of several methods that fall under a new "predicates" category, which would contain boolean-returning methods which check for particular qualities in a collection.

One other predicate I've been thinking of is something like someArray.allElementsAreEqual(), which e.g. would be true for [1, 1] but not [1, 2].

If we find other predicates to add to swift-algorithms, we can group ends(with:) among them.

Until then, I'm not sure which "guide" we should document this in.

natecook1000

Thanks, @amomchilov! Just a few small notes here.

natecook1000 · 2024-02-24T18:56:25Z

+  /// collection are equivalent to the elements in another collection, using
+  /// the given predicate as the equivalence test.
+  ///
+  /// The predicate must be a *equivalence relation* over the elements. That


Does this typo show up elsewhere in the project?

Suggested change

/// The predicate must be a *equivalence relation* over the elements. That

/// The predicate must be an *equivalence relation* over the elements. That

Not from swift-algorithms, but in the standard library itself.

I opened a fix: https://github.com/apple/swift/pull/71871/files

natecook1000 · 2024-02-24T18:57:14Z

+  ///     let a = 8...10
+  ///     let b = 1...10
+  ///
+  ///     print(b.ends(with: a))
+  ///     // Prints "true"


Could we use a string-based example here, instead of a range?

@natecook1000 I copied this doc from starts(with:) and modified it. I think if we change this, we should starts(with:) to match.

I'm good with either Arrays or Strings. Perhaps ranges are a bad example, given that some readers might confuse ..< and .... We can pick a more self-evident example that doesn't need to require knowledge of that distinction.

amomchilov · 2024-02-25T00:09:27Z

+  ///     let a = 8...10
+  ///     let b = 1...10
+  ///
+  ///     print(b.ends(with: a))
+  ///     // Prints "true"


@natecook1000 I copied this doc from starts(with:) and modified it. I think if we change this, we should starts(with:) to match.

amomchilov · 2024-02-25T00:09:37Z

+  ///   collection and the length of `possibleSuffix`.
+  // Adapted from https://github.com/apple/swift/blob/d6f9401/stdlib/public/core/SequenceAlgorithms.swift#L281-L286
+  @inlinable
+  public func ends<PossibleSuffix: BidirectionalCollection>(


Adapted from https://github.com/apple/swift/blob/d6f9401/stdlib/public/core/SequenceAlgorithms.swift#L281-L286

amomchilov · 2024-02-25T00:09:45Z

+  ///   collection and the length of `possibleSuffix`.
+  // Adapted from https://github.com/apple/swift/blob/d6f9401/stdlib/public/core/SequenceAlgorithms.swift#L235-L252
+  @inlinable
+  public func ends<PossibleSuffix: BidirectionalCollection>(


Adapted from https://github.com/apple/swift/blob/d6f9401/stdlib/public/core/SequenceAlgorithms.swift#L235-L252

amomchilov · 2024-02-25T00:17:20Z

+  /// collection are equivalent to the elements in another collection, using
+  /// the given predicate as the equivalence test.
+  ///
+  /// The predicate must be a *equivalence relation* over the elements. That


Not from swift-algorithms, but in the standard library itself.

I opened a fix: https://github.com/apple/swift/pull/71871/files

LucianoPAlmeida · 2024-03-09T21:58:36Z

+  @inlinable
+  public func ends<PossibleSuffix: BidirectionalCollection>(
+    with possibleSuffix: PossibleSuffix,
+    by areEquivalent: (Element, PossibleSuffix.Element) throws -> Bool


Since equivalence is dictated by areEquivalent does this need to be constrained to element is Equatable(BidirectionalCollection where Element: Equatable)?

Nice catch! I moved this overload into its own unconstrained extension.

https://github.com/apple/swift-algorithms/compare/17dfa21b45e1d3fdfd31e9f5e3a2c361f4a7cef6..f49094216b7f225dda7ee54ec7ad6c5801dfb75c

amomchilov · 2024-08-03T15:12:00Z

@natecook1000 Is this looking good to merge?

natecook1000 · 2025-01-14T18:23:11Z

@swift-ci Please test

amomchilov commented Feb 18, 2024

View reviewed changes

amomchilov force-pushed the ends-with branch from 1163301 to 8524728 Compare February 23, 2024 02:08

natecook1000 requested changes Feb 24, 2024

View reviewed changes

amomchilov force-pushed the ends-with branch from 8524728 to 375eb1f Compare February 25, 2024 00:17

amomchilov commented Feb 25, 2024

View reviewed changes

amomchilov requested a review from natecook1000 February 25, 2024 00:19

amomchilov mentioned this pull request Feb 25, 2024

Fix grammar in starts(with:) docs swiftlang/swift#71871

Merged

amomchilov force-pushed the ends-with branch from 375eb1f to 17dfa21 Compare February 25, 2024 03:44

LucianoPAlmeida reviewed Mar 9, 2024

View reviewed changes

Add ends(with:)

f490942

amomchilov force-pushed the ends-with branch from 17dfa21 to f490942 Compare March 10, 2024 15:26

amomchilov requested a review from LucianoPAlmeida March 10, 2024 15:27

LucianoPAlmeida approved these changes Mar 11, 2024

View reviewed changes

Naveen-C-Ramachandrappa approved these changes Oct 15, 2024

View reviewed changes

Merge branch 'main' into ends-with

d5942ca

natecook1000 approved these changes Jan 14, 2025

View reviewed changes

natecook1000 merged commit 401145b into apple:main Jan 14, 2025

	/// The predicate must be a equivalence relation over the elements. That
	/// The predicate must be an equivalence relation over the elements. That

Conversation

amomchilov commented Feb 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amomchilov Feb 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

natecook1000 Feb 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

natecook1000 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amomchilov commented Aug 3, 2024

Uh oh!

natecook1000 commented Jan 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

amomchilov commented Feb 18, 2024 •

edited

Loading

amomchilov Feb 25, 2024 •

edited

Loading

natecook1000 Feb 20, 2024 •

edited

Loading