Skip to content

When passing a collection to Set.contains(_:), forward to Set.isSuperset(of:) #796

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

Open
marcomasser opened this issue Jan 3, 2025 · 4 comments

Comments

@marcomasser
Copy link

Motivation

There is a confusing behavior of Set.contains(_:) when passing a collection that contains more than one element:

let s: Set = [1, 2, 3]
s.contains([1]) // true
s.contains([1] as Set) // true
s.contains([1, 2]) // false 🤨
s.contains([1, 2] as Set) // false 🤨
s.contains([1, 2, 3]) // false 🤨
s.contains([1, 2, 3] as Set) // false 🤨

I think this confusion is introduced by a Collection extension in the _StringProcessing module:

extension Collection where Self.Element : Equatable {

    /// Returns a Boolean value indicating whether the collection contains the
    /// given sequence.
    /// - Parameter other: A sequence to search for within this collection.
    /// - Returns: `true` if the collection contains the specified sequence,
    /// otherwise `false`.
    @available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *)
    public func contains<C>(_ other: C) -> Bool where C : Collection, Self.Element == C.Element
}

I understand that this behavior is caused by the Set internally storing its elements in a random order (as viewed from outside), so checking whether “[…] the collection contains the given sequence” is not really a useful thing to do on a Set, making this method pretty much useless there.

Proposed solution

To make this method useful on Set, it could simply forward to Set.isSuperset(of:), which is what I would have expected in the first place:

s.isSuperset(of: [1, 2]) // true
s.isSuperset(of: [1, 2] as Set) // true
s.isSuperset(of: [1, 2, 3]) // true
s.isSuperset(of: [1, 2, 3] as Set) // true

While maybe not strictly correct in the sense of the documentation that “[…] the collection contains the given sequence” because the order of elements as passed is irrelevant, I think it is more useful because the behavior is at least deterministic.

Alternatives considered

None really. Do nothing and keep the behavior as-is, I guess.

Additional information

I posted on the Swift forum about this and received only one reply that agreed with the confusing behavior.

Here’s the thread: Passing a Collection to Set.contains(_:) leads to confusing results

@marcomasser marcomasser changed the title When passing a collection to Set.contains(_:), forward to Set.isSuperset(of:) When passing a collection to Set.contains(_:), forward to Set.isSuperset(of:) Jan 3, 2025
@marcomasser
Copy link
Author

I also want to mention that this caused a bug in our app because of a line like this:

canvas.tree.elements.contains(interactionState.canvasInteractionState.selection.nodeIDs)

This worked as expected for an empty selection or single selection, but evaluated to false unexpectedly for multi-selections.

IMHO, the issue is that elements is a Set and while an isSuperset(of:) check is the correct thing to do here, it’s really easy to reach for the wrong method (contains(_:)) because (1) that method exists and (2) it’s valid to think of this as “I want to check if set A contains set B”.

I’m not familiar with set theory at all, but a quick check of the Wikipedia entry for Subset shows that saying “set A contains set B” seems to be a valid statement. At least in set theory. But not in Swift.

@sveinhal
Copy link

sveinhal commented Jan 6, 2025

let s: Set = [1, 2, 3]
s.contains([1]) // true
s.contains([1] as Set) // true
s.contains([1, 2]) // false 🤨
s.contains([1, 2] as Set) // false 🤨
s.contains([1, 2, 3]) // false 🤨
s.contains([1, 2, 3] as Set) // false 🤨

This is also non-deterministic and the output may change each time the program is run, depending on the seed used for hashing. s is treated as a collection, and its order is hash-dependent. So if Array(s) happens to return [1, 2, 3] (or some permutation with similarly-ordered subsets), the results will sometimes yield true, and other times false.

@marcomasser
Copy link
Author

That’s absolutely true, it seems I forgot to point out the non-deterministic behavior explicitly. Thanks for mentioning it!

@AnthonyLatsis
Copy link

@hamishknight I think we need to forward this to the string processing folks.

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

No branches or pull requests

3 participants