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

Evolution Proposal Reviews
1

The review of SE-0197 "Add in-place remove(where:)" begins now and runs through January 30th, 2018.

Reviews are an important part of the Swift evolution process. All reviews should be made in this thread on the Swift forums or, if you would like to keep your feedback private, directly in email to me as the review manager.

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  • 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?

More information about the Swift evolution process is available on the Swift evolution website.

As always, thank you for participating in Swift Evolution.

John McCall
Review Manager

9 Likes
2

What is your evaluation of the proposal?

+1. This will be a nice addition to the language.

One minor request is that I would like to see the proposal clarify the intended semantics regarding the state of the receiver is left in when an error is thrown.

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?

Very much.

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

N/A

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

A quick read.

3

What is your evaluation of the proposal?

+1

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

Yes, I've written many a filter remove.

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

Yes.

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

N/A

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

Read the proposal.

1 Like
4

+1

Yes, it's currently awkward to remove items in a similar fashion as in the proposal. My ad-hoc solutions are usually either inefficient code-wise or performance-wise.

Yes.

n/a

Quick read.

1 Like
5

What is your evaluation of the proposal?

+1. Adding this to the language will improve ease-of-use.

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

Yes, there are a number of particular cases where this will offer a significant improvement over the current approach.

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

The proposed addition reads quite well, in addition to being concise and intuitive.

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

N/A

How much effort did you put into your review?

I gave this proposal a quick reading and a minimal amount of research.

6

I am for the functionality of in-place filtering.

Proposal says:

The easiest way to achieve this effect in Swift 3 is to use filter and assign back, negating the thing you want to remove (because filter takes a closure of items to "keep")...this has two performance problems: fresh memory allocation, and a copy of all the elements in full even if none need to be removed.

It seems to me for the sake of parsimony that in-place filtering should use the same core name as the existing technology, such as filtered(where:) instead of reversing the logic and adding a new core name.

1 Like
7

What is your evaluation of the proposal?

This looks great. My only question is, are we going to have a remove(_ elem: Element) where Element: Equatable variant? That seems equally useful, and is well within our conventions.

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

Yes; it's an easy algorithm to mess up.

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

Yes; it has the name and semantics I would expect.

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

This is a very good Swift version of the design.

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

Just a glance.

1 Like
8

The Swift naming conventions tell us that these operations ought to named filter(where:) for the in-place operation and filtered(where:) for the copying operation. Unfortunately, that's the opposite meaning from how filter is used in every other language, including all extant versions of Swift. If you grant that we just have to accept that divergence, then it seems to me that using a new verb is the most appropriate choice.

2 Likes
9

headdesk. I mixed up the mutating/non-mutating rule.

I suppose applyFilter or filterInPlace would be uglier.

10

I agree with this in principal, but considering this usage xs.remove(42), it does not seem right to me to use just remove(_:) for this operation, since it does not make it clear whether all the 42s will be removed, or just first one of them, or anything else. removeAll(_:) or remove(elementsEqualTo:), perhaps...

/cc @Ben_Cohen, I bet it was mentioned once or twice during the mailing list discussion ;-)

1 Like
11

Unlike contains and similar methods, I don't think remove(_ value: Element) is all that useful a feature, so probably not worth spending the API real estate on it. I guess the few cases it would be useful is remove(0) and remove(nil).

(filter(_ value: Element) is even odder, since it's basically saying "repeat value the number of times it occurs in self).

1 Like
12

I'm sure s.remove(" ") would be a hit.

1 Like
13

They could also use remove(where:) with a isWhitespace predicate (which might be built-in to Character or something).

Would an index-taking variant also be feasible? Removing a bunch of indexes from a (RangeReplaceable)Collection is also very common and very easy to get wrong.

Otherwise +1. The standard library's modelling of removing things from Collections is very basic. This is a function users would expect to find and we should have it.

2 Likes
14

This is definitely a good API to add—the proposal and my experience clearly show how this is in the “easy to get wrong” camp, and even when I implement this correctly on the first try, I generally don’t do it the quickest way.

Re: the name—some of the comments have made me wonder if it’s totally clear that this doesn’t only remove the first element that matches the predicate. Generally, our collection methods that operate on multiple items have some kind of hint in the name (removeAll, removeSubrange, insert(contentsOf:), etc), while the two methods with a where parameter label (contains(where:) and first(where:)) both short-circuit on the first match.

Calling it removeAll(where:) would solve this problem (if it’s deemed a problem), and would be a better stem if anyone did want to add removeAll(_: Element) for equatable collections to their own libraries.

var numbers = Array(1...10)
numbers.contains(where: { $0 % 2 == 0 })  // true
numbers.first(where: { $0 % 2 == 0 })     // 2
numbers.remove(at: 9)
numbers.removeSubrange(5...)
numbers.removeAll(where: { $0 % 2 == 0 }) // new!
numbers.removeAll()
7 Likes
15
  • What is your evaluation of the proposal?

It should be added, modulo: (1) the name, base and argument, be modified to indicate that multiple elements are targeted, not just the first; (2) the Equatable variant is added at the same time to match similar methods.

  • 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

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

N/A

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

Quick reading

16

I’m sure s.remove(" ") would be a hit.

Ah yes, maybe. Though maybe s.remove { $0.isWhitespace } could be similarly popular.

(or even s.remove(where: \.isWhitespace) someday... but I'll keep my dreams of gratuitous syntactic sugar for another time)

3 Likes
17

Thanks Nate. Putting it in context with the other removes does seem to suggest removeAll(where:) could be a better name.

18

I also think including a removeAll(where predicate: (Index, Element)->Bool) variant would be useful.

There are some StackOverflow links here and here showing some poor implementations of removing multiple indices for anyone interested.

1 Like
19

What is your evaluation of the proposal?

+1

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

Yes, I've been writing such remove(where:) extensions pretty much in every project I worked on. Having this in stdlib would be great!

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

Definitely.

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

N/A

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

A quick read of the proposal.

20

What is your evaluation of the proposal?

+1 for the functionality.

I agree with Nate’s argument for naming it removeAll.

I’m also bugged by the inconsistency that’s bugging Erica; filterInPlace would also make sense (with the condition swapped, obviously). However, John’s reply makes sense to me.

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?

It’s an obvious and annoying oversight in the stdlib that I’ve repeatedly had to implement myself, probably ineffeciently.

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

Ruby provides a method quartet (mutating / non-moutating; true keeps / true removes) for this:

select / select!
reject / reject!

(The exclamation point in Ruby is part of the method name, and by convention indicates mutation or other “dangerous” methods.)

It’s surprisingly helpful for code readability to have both select and reject in the toolbox. It allows concise method refs where a closure would otherwise be required, and more importantly the name difference is often more conspicuous than the not operator. They help code read better. (I like to alias contains(where:) as any / all for similar reasons.)

However, given Swift’s precedent of using filter, I don’t see a similar name quartet that reads as nicely. In a green field, I might propose this Swifty parallel to Ruby:

selectingAll(where:) / selectAll(where:)
removingAll(where:) / removeAll(where:)

…or maybe:

keepingAll(where:) / keepAll(where:)
removingAll(where:) / removeAll(where:)

…but this is a far departure, and I don't think it warrants a change to the library. The duo (not quartet) named as proposed seems good enough to me, if not ideal.

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

A glance.