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

[7.x] Refactor contain matcher using Predicate.simple #573

Merged
merged 2 commits into from
Jul 18, 2018

Conversation

ikesyo
Copy link
Member

@ikesyo ikesyo commented Jul 11, 2018

Replacing Predicate.fromDeprecatedClosure.

@ikesyo ikesyo requested a review from a team July 11, 2018 18:44
if let actual = try actualExpression.evaluate() {
return items.all {
let matches = items.all {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not necessary in this PR, but wondering if we can rename this to allSatisfy and make it a compatibility shim for Swift < 4.2? https://github.com/apple/swift-evolution/blob/master/proposals/0207-containsOnly.md

Copy link
Member Author

Choose a reason for hiding this comment

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

Totally makes sense 👍

return items.all { item in
return Predicate.simple("contain <\(arrayAsString(items))>") { actualExpression in
guard let actual = try actualExpression.evaluate() else { return .fail }
let matches = items.all { item in
return item != nil && actual.contains(item!)
Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on restructuring this to avoid the force-unwrap?

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in 9bbe098.

@ikesyo ikesyo force-pushed the refactor-contain-matcher branch from 380f957 to 9bbe098 Compare July 18, 2018 04:38
@ikesyo ikesyo merged commit b667637 into 7.x-branch Jul 18, 2018
@ikesyo ikesyo deleted the refactor-contain-matcher branch July 18, 2018 06:57
Megal pushed a commit to Megal/Nimble that referenced this pull request Jul 31, 2019
[7.x] Refactor `contain` matcher using `Predicate.simple`
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