removeAll(closure) in Swift

Question

From this library I wrote, I have created this function in Swift, but I'm not happy with the implementation. Can anyone suggest a better way to do this? I really don't like changing counters inside for-loops...

extension Array {
 /// Removes all items that conform to the given closure
 mutating func removeAll(closure: (T) -> Bool) {
 for var counter = 0; counter < count; counter += 1 {
 if closure(self[counter]) {
 removeAtIndex(counter)
 counter -= 1
 }
 }
 }
}

Answer 1

Some general remarks:

• The method name removeAll is misleading, as it does not remove all elements from the array. I would call it for example removeMatching.
• The method parameter is a closure, but acts as a predicate, so predicate might be a better name.
• The counter variable in the method is actually the array index so you could call it index.
• The method documentation can be improved (details at the end of this answer).

Your concrete problem about modifying the loop counter inside the loop can easily be solved by traversing the array indices in reverse order:

for var index = count - 1; index >= 0; index-- { ... }

Using stride() allows you to have a constant loop variable:

for index in stride(from: count - 1, through: 0, by: -1) { ... }

Putting it all together:

extension Array {
 /// Removes all items that satisfy the predicate:
 mutating func removeMatching(predicate: (T) -> Bool) {
 for index in stride(from: count - 1, through: 0, by: -1) {
 if predicate(self[index]) {
 removeAtIndex(index)
 }
 }
 }
}

An alternative implementation would be

extension Array {
 /// Removes all items that satisfy the predicate:
 mutating func removeMatching(predicate: (T) -> Bool) {
 self = self.filter { !predicate(0ドル) }
 }
}

The first method removes the matching elements, while the second method creates a new array and then replaces self. Which one is more efficient depends how many elements are actually removed in relation to the array size.

Notice that in Swift 2, Element should be used in place of T.

As mentioned above, the inline method documentation can be improved (I took this information from the SO thread Swift Documentation Comments). Example:

extension Array {
 /// Removes all items that satisfy the `predicate`
 ///
 /// :param: predicate A boolean predicate.
 mutating func removeMatching(predicate: (T) -> Bool) {
 self = self.filter { !predicate(0ドル) }
 }
}

Now "Option-Click" will show you

enter image description here

---

Addendum: A removeAll(where:) was added to the Swift Standard library in Swift 4.2, so there is no need to define your own method for that purpose anymore.

As explained in SE-0197 this method is supposed to be both safe and efficient.

• vrwim: I agree with all points, especially the last using filter, but what is the advantage of using stride()?
• vrwim: Ooh I was looking at using better documentation, and I was looking at @param, but that didn't seem to work, thanks!
• Martin R: @vrwim: The advantage is that index is a constant inside the loop so that you cannot inadvertently modify it. Using a constant may also allow better compiler optimizations.
• vrwim: @MartinR ok, thanks! I will test out the filter vs my implementation for speed and I'll get back to you
• vrwim: Holy moly; When using this code (where removeMatching is your last code), I get this result, which is like 4-5 times slower.