SE-0207: Add a containsOnly algorithm to Sequence

Evolution Proposal Reviews
1

The review of SE-0207: Add a containsOnly algorithm to Sequence begins now and runs through April 13th, 2018.

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to avoid this forum or just keep your feedback private, directly to me via email, twitter, or direct message. If emailing me directly, please put “SE-0207: Add a containsOnly algorithm to Sequence” in the subject line. Your feedback is equally valuable to us no matter how we receive it.

What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

What is your evaluation of the proposal?

Is the problem being addressed significant enough to warrant a change to Swift?

Does this proposal fit well with the feel and direction of Swift?

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,
Dave Abrahams
Review Manager

4 Likes
2

  • What is your evaluation of the proposal?

    Good idea

  • Is the problem being addressed significant enough to warrant a change to Swift?

    Yes - though not overwhelmingly yes - I have used the negated contains before

  • Does this proposal fit well with the feel and direction of Swift?

    Yes - more important for newbies too Swift.

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

    Sort of. Scala has equivalent of count(where predicate: (Element) throws -> Bool) rethrows -> Int which could be used as list.count { $0 == 9 } == list.count.

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

    Quick read

3

The functionality is useful and fits with the existing API. However—in the grand tradition of Swift Evolution—I am not sold on the name containsOnly(where:). I spent some time pondering alternatives, and came up with two that seem plausible:

• allMatch() — allEqual()
• entirely() — isEntirely()

For comparison, the proposal and the one alternative it mentions are:

• containsOnly(where:) — containsOnly()
• all() — all()

Here is how these four options look in different scenarios:

// Trailing closure — check for straight-A student
if (grades.allMatch{ $0 >= 90 }) { … }
if (grades.entirely{ $0 >= 90 }) { … }
if (grades.containsOnly{ $0 >= 90 }) { … }
if (grades.all{ $0 >= 90 }) { … }

// Predicate function — check for blank string
if text.allMatch(isWhitespace) { … }
if text.entirely(isWhitespace) { … }
if text.containsOnly(where: isWhitespace) { … }
if text.all(isWhitespace) { … }

// Equatable element — check for the origin
if coordinates.allEqual(0) { … }
if coordinates.isEntirely(0) { … }
if coordinates.containsOnly(0) { … }
if coordinates.all(0) { … }

In the first and third examples, all these wordings are reasonable. However, in the middle example two of them are quite awkward:

text.allMatch(isWhitespace)
text.containsOnly(where: isWhitespace)

For allMatch, the problem is specific to String, as the name of the variable does not evoke being a Sequence. For containsOnly(where:) though, the problem is broader and stems from the argument label.

I am not sure what spelling I prefer, but I think we should consider alternatives.

2 Likes
4

I’m +1 to this. I use methods like this in other languages and find them very useful.

After reading the original thread I’ve been swayed from the term ‘all’ and think containsOnly is a better name for this.

5

Hm I do agree that the where in the predicate version looks a bit odd when you just supply a variable. But I think thats fine in the case where you’re supplying a closure directly. None of the alternatives I can think of are really much better.

Although I will say, when I started thinking of alternatives, the best and first that came to mind is one you pointed out. allEqual. So I think if we do consider alternatives, that is a strong one.

6

containsOnly has the small possibility that someone may think that

[1].containsOnly(1) // true
[1, 1, 1].containsOnly(1) // false?

I don't see it as a big danger, but allEqual leaves no room for such misinterpretation.

But imho we have to be careful with adding small helpers to the stdlib - if we had a dedicated library for collections, I'd probably put all extensions that aren't already included into such a package (together with some less common datatypes).

1 Like
7

I definitely wouldn't mind spinning another discussion off of this one to discuss adding a Collections framework that would either ship with the toolchain, or be a package importable through SPM. Might even want to wrap the linked list proposal in that framework.

9

+1 from me too.

I've had to write this functionality in an extension more times than I can count.

I'm not too enthusiastic about the name containsOnly(). Would prefer seeing all() or allMatch() as mentioned above.

10

I really like this proposal since I often find myself writing a negation of contains and it takes a little bit of time every time I have to read again the code because of its suboptimal readability. I also agree on the naming containsOnly since it really fits in with the existing contains. Adding this new method with a completely different name compared to the existing one would definitely create some confusion.

11

+1 to the functionality, -0.5 to the name.

The proposed names are … acceptable, but not satisfying. Their asymmetry feels awkward to me. The terminology of contains(where:) has always felt a bit off when I use it, and feels more off now.

I often implement this pair myself as any(match:) and all(match:) simply because I find they read better in context. Here is an example of that.

One could quibble with the details of that particular naming — this is Swift evolution, after all! — but I would add my voice to those strongly in favor of using the words any and all in some form as the distinguishing feature of these two methods. The pair are a widely understood term of art:

They have worked well in practice in all these languages — for at least a decade in the case of Python and Ruby!

Their wide usage is argument enough, I think. But their ubiquity is in this case a sign of good design: they communicate the right idea even to someone not yet familiar with them, and do not require the extra step of mental indirection that contains / containsOnly do.

6 Likes
12

I'm happy to see this make it to a proposal! I participated in the original discussion and supported the proposed naming. I don't have much to add beyond that discussion, but here's a summary of my reasoning for choosing containsOnly:

  • The alignment with contains makes it easy to remember, and it's easily discoverable via autocomplete when you try to use the inverted contains (i.e. collection.contains { !expression }).
  • It is one of the only suggested names that satisfies this API design guideline: "Uses of Boolean methods and properties should read as assertions about the receiver". Following this rule is what makes containsOnly the most natural-sounding option to me.

Of the alternatives, I'm not a huge fan of all in particular because it looks to me like an alias of filter, i.e. integers.all { $0.isEven }

1 Like
13

To be clear, I’m only voting in favor of the prefixes “any” and “all,” not necessarily in favor of those being the entire method name.

Variants such as anyMatch / allMatch also satisfy that guideline, and also have the advantage of satisfying this one:

Embrace precedent. Don’t optimize terms for the total beginner at the expense of conformance to existing culture.

At this point, per the links above, the programming community precedent for names starting with any* / all* is widespread and longstanding.

That said, if bringing contains(where:) into alignment with the rest of the programming world is not on the table, using an “all-” prefix without a corresponding “any-” doesn’t feel right.

FWIW, I spent an embarrassingly long time without even realizing that contains(where:) existed at all, and reimplementing it on every project. Perhaps this is just my own uniquely addled brain, but that name was not easily discoverable for me in actual experience; I’m skeptical that containsOnly would be.

1 Like
14

This is maybe getting a little too far into the weeds, but I wouldn't consider them to satisfy the guideline because they read as assertions about the /contents/ of the receiver rather than the receiver itself. That sounds like a minor distinction, but I think it creates a grammatical issue.

Consider if array.containsOnly(5) vs. if array.allMatch(5). The former reads naturally on its own as "if array contains only 5", while the latter requires inserting some additional implied words to read naturally: "if [the elements of] array all match 5".

That said:

This is certainly a good argument for all or allMatch. It's likely the name that developers coming from the languages you listed will initially reach for. I do agree though that it would be unfortunate to use that name without renaming contains to any or anyMatch, and that would be pretty heavily source-breaking.

15

Consider if array.containsOnly(5) vs. if array.allMatch(5). The former reads naturally on its own as “if array contains only 5”, while the latter requires inserting some additional implied words to read naturally: “if [the elements of] array all match 5”.

Meta-point: when doing these readability experiments, it's important not to use variable names that merely parrot their types, so I'd look at

if gradePointAverages.containsOnly(5)

vs.

if gradePointAverages.allMatch(5)

-Dave

1 Like
16

FYI, what I suggested above would only use allMatch with a predicate. When taking an Equatable element it would be spelled allEqual. Adopting Dave’s suggestion, that becomes:

if gradePointAverages.containsOnly(5)
if gradePointAverages.allEqual(5)

With a trailing closure it would be allMatch:

if (gradePointAverages.containsOnly{ $0 > 4 })
if (gradePointAverages.allMatch{ $0 > 4 })

And also with a predicate function:

if gradePointAverages.containsOnly(where: isPassingGrade)
if gradePointAverages.allMatch(isPassingGrade)

An alternative to allMatch would be allSatisfy, though personally I prefer allMatch.

I think it is fine to leave the existing contains as-is, regardless of how we spell the new method.

1 Like
17

That's a good point that I hadn't considered. allMatch does read better when the variable is a pluralization of the elements, which is usually the case for a collection.

For what it's worth, existing collection APIs like isEmpty read better when the variable is named after the container rather than the elements, i.e. array.isEmpty vs. gradePointAverages.isEmpty. But both naming conventions are used, and it's impossible to accommodate both perfectly. At this point, I've seen enough tradeoffs in either direction that I'd be happy with either set of names.

18

Re the all family of names: one of the clear signals from the SE-0204 thread (adding lastIndex(where:) is that people really don't like mismatched APIs. Presumably this would also go for having all/contains rather than all/any.

But we really really cannot be renaming contains at this stage, purely so that we can introduce a new related method.* If you value the symmetry, this pretty much locks us in to containsOnly instead.

Personally, I don't find the symmetry all that critical, so I'd be fine with ending up with all/contains (hence that was in the original proposal). But at the same time I don't feel that all or allMatch are any better than containsOnly.

* the rename of index(where:) is a little different, since it was a clear outlier from the existing first/last family, and index(where:) didn't really fit in with it's siblings index(after:) etc which are all about index advancement, not search.

7 Likes
19

Is it a correct English interpretation of “if array contains only 5” to make it match only [5]?

20

Correction: it has been pointed out to me that this review should run through the 13th, not the 6th as originally stated. I've updated the announcement accordingly.

21

I would say yes.