SE-0207: Add a containsOnly algorithm to Sequence


(Lance Parker) #83

This is a great point. The method in question returns a Bool, which should make it obvious that this is not doing a filter operation.

It’s also worth noting that you can’t use trailing closure syntax in the condition of an if statement. The closure must be in the parentheses of the function call.

if array.all(match: { $0 % 2 == 0 }) { // works
  print("All even")
}

if array.all { $0 % 2 == 0 } { // Error: Trailing closure requires parentheses for disambiguation in this context
  print("again")
}

The second example above gives you a fixit that puts back in the argument label for the closure.


(Jon Shier) #84

To single out that single guideline at the expense of the others is very bad design. Neither any nor all work well with trailing closures, meaning they violate most of the rest of the guidelines about clarity or fluency. Besides which, I would interpret “Don’t optimize terms for the total beginner at the expense of conformance to existing culture.” as to be talking about Swift culture first, programming culture in general second.


(Lance Parker) #85

Can we come up with an example where trailing closure calls to all would be confusing, or is this just hypothetical confusion? I can’t think of any examples where the return type of all being a Bool wouldn’t signal to the reader what is actually happening.


(Pavol Vaskovic) #86

I think it’s hypothetical. The trailing closure array.all {...} can not be mistaken for a filter, because Swift is a strongly typed language. Looking at the method’s signature that returns a Bool, you can not reasonably confuse it for a filter operation that would have to return [T]. As the Guidelines state:

Brevity in Swift code, where it occurs, is a side-effect of the strong type system and features that naturally reduce boilerplate.

When designing framework-type API, that is used by others, as opposed to the application code where you’re in full control, you have a leverage only over half of the readability equation. The user of your API has to meet you halfway and do her part in ensuring clarity at the use site. In this case it means that programmers should reach for the trailing closure syntax only when it makes their code easier to read. They still have the option, to define nested function, to give the predicate closure a descriptive name and use it with argument label:

if gradePointAverages.all(satisfy: passingGrade)

Choosing between an inline closure or a named function is judgment call that rests with the user of your API. Its her responsibility to ensure clarity and readability at the use site. Our responsibility here is to create API that enables it. We can mess up by not designing a fluent interface, but there’s no way we can force readability by adding more words to the method name. All things being equal, we should prefer short names, because adding unnecessary filler words actually hinders readability, even when used well. You can not get back more brevity at the use site, once we choose a method name that is mile long.

To illustrate, this is Scala code I wrote some 8 years ago. I’ve used inline closure in .groupBy(_.fullName) because its perfectly clear, but I’ve named the sorting function moreFirst and the mapping function countOccurrences to build up vocabulary for my little embedded domain specific language. That allows me to get a very dense yet expressive result, because Scala collections do have fluent interface (even without argument labels).


(Jon Shier) #87

all is less likely to be confusing with a trailing closure, primarily because the user has some context for what it does given that “all” can only really mean “all elements” (if the user knows the callee is a collection). Given the return type is invisible, it’s of secondary help, as the user would only look at it when confused about the method name. any is far worse, as there are multiple meanings, which would require the user to look at the return type, at which point they’re just viewing the documentation in general. To my eye, contains is strictly superior there since it not only conveys the any-ness of any, but implies it’s based on the contents of the collection and not anything else. Given that, containsOnly has the same advantages, in addition to symmetry with the existing method.


#88

There has been recent feedback from the core team that says quite clearly that API renaming is unlikely to happen unless strong evidence of actual harm: contains(e) and contains(where: predicate) are unlikely to be replaced by any any variant.

That being said, I’d like to add my voice for all(satisfy:), and only this method:

  • The “are all elements of this sequence equal to this value ?” question is likely to be wanted by nobody, as revealed by Paul’s cross-language survey. Swift and R would be the only language to have a dedicated API for it. We don’t need this method.
  • Thus we only need a predicate-testing method, equivalent to !contains(where:!predicate)
  • if urls.all(satisfy: { $0.host != nil }) { ... } is pretty fluent
  • let qualified = urls.all { $0.host != nil } looks pretty readable to me too
  • I’d really like that we consider the power of documentation, culture, and frequent use to settle habits in the community of Swift users. Soon enough, all would just enter the common Swift vocabulary, just like other totally made-up names like map did.

(Jon Shier) #89

It seems mostly like a shortcut for containsOnly { $0 == 1 }, which seems like a logically common usage, at least for Equatable types. I consider it roughly equivalent to flattened() or compacted().

I don’t find them much more readable than if urls.containsOnly(where: { $0.host != nil }) { ... } or let qualified = urls.containsOnly { $0.host != nil } (which looks more readable to me). And you lose the symmetry with contains, which isn’t likely to be renamed, like you said.

You can say that about literally any bad API, so “they’ll get used to it” isn’t really a good argument for any particular name. Besides, documentation should serve to clarify naming, not as a crutch for poor names to make their way into the language. The only reason map and reduce are named what they are is 1) they came with the language before it was open source, and 2) their status as a term of art, not just from other languages but from language theory in general. I don’t find those to be very good names, and I think we’d have been better served having more natural names, but this is where the value equation came out.


#90

Yes. The predicate version is enough.

I didn’t pretend to be objective. And I don’t care about this symmetry. It brings nothing.

Yes. But it’s a good argument for good APIs.

No. I mean, not always. Your sentence does not apply to map or flatMap. Or union/formUnion. Those oddities that have grown in us.

The survey above gives some historical background to all.

I agree, again. It just happens that all is more “natural” to me than containsOnly. We don’t have to pretend we’re always objective. There will be a subjective choice, eventually. I hope it will not be guided by a symmetry that has no value other than its mere existence (unless I’m mistaken).


(Dave DeLong) #91

My apologies if this has been brought up before, because this is a HUGE thread and I haven’t been able to keep up with all of the stuff getting proposed.

I was wondering if anyone has considered adding both all(match:) and containsOnly(_:) methods, and then have them both just do the same thing; one would forward to the other.

Why%20not%20both


(Nobody1707) #92

Although the second one does work if you wrap the whole condition in parenthesis:

if (array.all { $0 % 2 == 0 }) {
    print("again")
}

(David Hart) #93

Ruby has this convention of functions with different names that forward to a single implementation and I’ve always found this very confusing. When I see two functions with different names, I naturally assume that they are named differently because they have different semantics, and Swift has stuck to that until now.


(Lance Parker) #94

Even so, it’s immediately apparent that this function returns a Bool


(Benjamin Mayo) #95

I don’t like the ‘obvious from type context’ arguments, at least I think it is not a rock-solid motivation. Swift guidelines strongly indicate that boolean properties are named as assertions; isHidden rather than hidden. Even though both return a Bool, it’s so much easier to understand what the code means when you read it.

Plus, it’s important not to neglect the benefits of consistency with existing standard library and idiomatic Swift framework APIs. If I see contains in the method name, I’m thinking this operation tests the collection with some predicate, somehow. No ifs or buts about it. Simple as that.

The best argument for all is familiarity argument and term-of-art significance. I don’t discount the importance of that either. It’s my personal opinion that this term is not so fundamental that it should override every other rule in the guidelines. Obviously, it’s up to the core team to make the judgement call here.

Tangentially, I liked the idea up-thread to remove containsOnly(_ element: Equatable) from the proposal and only implement containsOnly(where:). The former is the root of the ambiguity I think, and probably doesn’t hold its weight - being trivially implemented in terms of containsOnly(where:). It can always be introduced at a later stage, perhaps with an independent name, if in demand.


#96

This is too inconsistent with contains(_:) and contains(where:), especially if the name containsOnly is used.


(Tim Vermeulen) #97

let foo = array.all { ... }

When you read a line of code it’s not always immediately obvious which types are involved. Of course it’s always possible to figure out that it’s a Bool by looking at how foo is used later on, or perhaps by the actual variable name of foo, but that doesn’t mean that reading a line like this one can’t be confusing.

In my experience, type inference generally makes Swift code very readable because more often than not it’s immediately obvious which type a method returns based on its name. Introducing method names that are ambiguous until you look at the return type would then be pretty counterproductive.


(Dave Abrahams) #98

FWIW, I read

x.whereAllElements(satisfy: isOdd)

as an expression that is equal to x if all its elements are odd, and—I suppose—nil otherwise.

Also the is prefix is not ever “preferred” unless it happens to be the best way to meet the guideline quoted above. It often is, so we use it, but if using it would require contortions, that’s a pretty good indicator that is is not the best route to meeting the guideline.


(Dave Abrahams) #99

When I wrote this I had forgotten that the people developing Foundation’s Swift API have discovered, since the guidelines were released, that they needed some flexibility beyond the guidelines in choosing where to break phrases into argument labels. It’s possible they have enough experience at this point that we can update the guidelines, and that the update would justify the choice above. @Tony_Parker do you have anything to add here?


(Tony Parker) #100

I think what you’re referring to is a rule that methods that are not central to the purpose of the type should not have very short names. I think one of our canonical examples was NSView and its subviews. NSView does have subviews, but its primary purpose is not its subviews, therefore a method name like addSubview is preferred to add. (side note: One could also make an argument that a subview is a specific kind of view and therefore deserves a more specific label anyway, but this is still a useful thought experiment for the ‘central purpose’ rule).


(Dave Abrahams) #101

Thanks @Tony_Parker, but I think I was referring to something else: I seem to remember some decisions to break usage that contains a prepositional phrase into basename and label at a word preceding the preposition, rather than at the preposition itself…?


Bikeshedding Names Considered Harmfull
(Dave Abrahams) #102

Summary

This review ended April 13th. The core team accepted one of the two proposed Sequence extensions, with a different name than was proposed:

extension Sequence {
  /// Returns a Boolean value indicating whether every element of the sequence
  /// satisfies the given predicate.
  func allSatisfy(_ predicate: (Element) throws -> Bool) rethrows -> Bool
}

Thanks to all who participated for making Swift a better language!

Details

Consensus among core team members was that:

  • The name containsOnly
    • is too easily misread at the use-site as “contains one instance equal to.”
    • is especially confusing when applied to empty sequences, considering that the result must be true.
    • Does not fully address the cognitive difficulty that comes from trying to express these semantics in terms of contains.
  • The name allSatisfy makes usage completely clear.
  • Breaking the base name into all(satisfy:)
    • is not a precedented pattern in our names
    • is not supported by anything in the API guidelines
    • was considered by some to result in unclear usage with trailing closures, e.g. x.all { $0 > 5 }
  • The form of containsOnly that checks all elements of the Sequence for equality to its argument, e.g. x.containsOnly(5)
    • is rarely useful in practice
    • can easily be written as x.allSatisfy { $0 == 5 }
    • can always be added later as x.allEqual(5)