HiveBrain v1.2.0
Get Started
← Back to all entries
patternswiftMinor

removeAll(closure) in Swift

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
swiftremoveallclosure

Problem

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
            }
        }
    }
}

Solution

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

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.

Code Snippets

for var index = count - 1; index >= 0; index-- { ... }
for index in stride(from: count - 1, through: 0, by: -1) { ... }
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)
            }
        }
    }
}
extension Array {

    /// Removes all items that satisfy the predicate:
    mutating func removeMatching(predicate: (T) -> Bool) {
        self = self.filter { !predicate($0) }
    }
}
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) }
    }
}

Context

StackExchange Code Review Q#86581, answer score: 7

Revisions (0)

No revisions yet.