SE-0197 — Add in-place remove(where:)

Evolution Proposal Reviews
21

+1 for removeAll(where:) for the reasons noted by Nate (@nnnnnnnn) and others.

22

I'm with @Paul_Cantrell. I'd prefer to add four new keywords that follow the Swift API guidelines than try to fight an industry term of art to wrestle the right behavior into the language by pairing filter with removeAll. (Thanks @nnnnnnnn for the improved naming btw.)

This isn't quite a "if you were designing it from scratch" situation but it's close. And in such case, the utility of positive and negative logic versions is evident. I'd want to filter a sequence but maybe I'd prefer to selectAll , selectingAll, removeAll, removingAll an array, set, or dictionary.

23

The proposal mentions concrete implementations for Set and Dictionary, but those do not seem to be part of the implementation. Are those coming as well? This seems especially useful for Dictionary:

dict.remove[All] { isOdd($0.value) }

24

Good point. I'll add them to the PR.

25

What is your evaluation of the proposal?

+1

The following point is a bit orthogonal to the performance aspect, or improving the readability of filter meaning "keep", but I wanted to point out that if all you're looking to do is change a "keep" filter into a "remove" one then the readability of nums = nums.filter { !isOdd($0) } could be slightly improved by extending the ! operator to act on Bool returning functions, e.g.

prefix func !<D>(_ fn: @escaping (D) -> Bool) -> (D) -> Bool { return { !fn($0) } }

Then the example reduces to:

var nums = [1,2,3,4,5]
// remove odd elements
nums = nums.filter(!isOdd)

In-fact this may be useful for avoiding writing a closure when using this new in-place remove(where:) when simply the negation of a function is desired.

To remove even numbers, you could write nums.remove(where: !isOdd) for example.

That said, I think there is enough value in the performance aspect, and the convenience of being an in-place removal if dealing with an already mutable collection to warrant adding this new method.

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

Yes.

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

Yes.

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

A couple of quick reads though.

2 Likes
26

Any more comments before review concludes? Currently it seems like consensus is in favor in adding the operation, but spelled removeAll(where:).

4 Likes
27

+1 on the proposal and I also support the spelling removeAll(where:), as the method is basically a constrained version of removeAll().

Once SE-0174 lands, could this method also be added to Sequence where Filtered == Self? That should cover Dictionary and Set and be useful for some user-defined types as well.

1 Like
28

The review of this proposal is now over. The core team has decided to accept the proposal with the revision that the method be named removeAll(where:).

In this thread, we also discussed whether to add a removeAll(_: Element) method. While I think that's a reasonable-sounding idea, I do think it can considered independently of SE-0197, since it wasn't originally part of the pitch. If you're interested in adding that method, I encourage you to start a new pitch thread for it.

1 Like
29

I'm a bit late for this one, Just want to point out that we now have:

drop(while:)
and the new
removeAll(where:)

both are somehow related and I'd love it we'll eventually, at some point, end up with drop(where:) too, or removeAll(while:)

30

Sequence.drop(while:) walks the elements in order and stops at the first disqualifying one, while Collection.removeAll(where:) conceptually checks all elements and ignores their iteration order. So, giving each method an overload for the other one's mode wouldn't make sense.

1 Like
31

I think @krzyzanowskim just wanted to say that we have two different verbs for methods that get rid of elements -- and probably was fooled by one name, which isn't dropping(while:) as you might expect because of our rules.
I think both names make sense in isolation, but it seems arbitrary that one operation exists only in a form that creates a new object, and the other only has a mutating variant.

32

thanks, that was exactly my point. thanks!

I understand that English grammar is important, but sometimes I feel like Swift is one step too far with the naming ;)

33

I hate to bump an old thread, but seems like removeAll(where:) is missing in both Dictionary and Set as of 5.3. Cannot find the discussion of why their implementations were dropped.

34

Were they ever on Dictionary and Set? The proposal talks about RangeReplaceableCollection which those two are not.

35

It does look like they were in the proposal but omitted from the implementation:

Since Dictionary and Set would benefit from this functionality as well, but are not range-replaceable, they should be given concrete implementations for consistency.

We recently adopted rules that such missing parts of implementations would expire after a year. I don't recall if there was a stumbling block in implementing these or if it was just missed. I'll check and if it's a simple omission, should be an easy re-propose.

1 Like