Skip to content
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

adding beginWith matcher for sequence prefixes #719

Merged
merged 1 commit into from
May 22, 2020
Merged

Conversation

Skoti
Copy link
Contributor

@Skoti Skoti commented Dec 28, 2019

Hi I would like to add a convenience matcher for sequence prefixes.
So that we can expect a particular sequence to start with provided elements.

Checklist - While not every PR needs it, new features should consider this list:

  • Does this have tests?
  • Does this have documentation?
  • Does this break the public API (Requires major version bump)?
  • Is this a new feature (Requires minor version bump)?

@ikesyo
Copy link
Member

ikesyo commented May 18, 2020

Thanks for the pull request! While this should be useful, I think the name would confused with the existing beginWith matcher. How about making it beginWith overload?

@Skoti
Copy link
Contributor Author

Skoti commented May 18, 2020

I could, however, this case:

let y = [[Int]]()
expect(y).to(beginWith([]))

won't work unless you specify the type explicitly:

expect(y).to(beginWith([] as [[Int]]))

without explicit type cast it calls this:

public func beginWith<S: Sequence, T: Equatable>(_ startingElement: T) -> Predicate<S>
where S.Iterator.Element == T {

and fails with a hard to understand error (at least at first glimpse):
failed - expected to begin with <[]>, got <[]>.

also this case won't work for the same reason:

let y = [Int]()
expect([]).to(beginWith(y))
// or hypothetically
expect([]).to(beginWith([] as [Int]))

unless:

expect([] as [Int]).to(beginWith(y))
expect([] as [Int]).to(beginWith([]))

So to avoid confusion and add more convenience at call site, in these and maybe some other cases, I have named it startWith.

@ikesyo
Copy link
Member

ikesyo commented May 20, 2020

Fair point, thanks for pointing it out. But I'm still uncomfortable about beginWith/startWith coexistence.

Let's see Sequence.starts(with:) signature:

func starts<PossiblePrefix>(with possiblePrefix: PossiblePrefix) -> Bool where PossiblePrefix : Sequence, Self.Element == PossiblePrefix.Element

I propose beginWith(prefix:) for this matcher. The "prefix" word would be suitable in connection with PossiblePrefix/possiblePrefix names and Collection.prefix(_:) API.

Copy link
Member

@ikesyo ikesyo left a comment

Choose a reason for hiding this comment

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

Just a few minor comments but entirely looks good 👍

Sources/Nimble/Matchers/BeginWithPrefix.swift Outdated Show resolved Hide resolved
Sources/Nimble/Matchers/BeginWithPrefix.swift Outdated Show resolved Hide resolved
Sources/Nimble/Matchers/BeginWithPrefix.swift Outdated Show resolved Hide resolved
@ikesyo ikesyo changed the title adding StartWith matcher for sequence prefixes adding beginWith matcher for sequence prefixes May 21, 2020
Copy link
Member

@ikesyo ikesyo 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 your contribution!

@ikesyo ikesyo merged commit ab91540 into Quick:master May 22, 2020
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.

2 participants