CODEWITHSUNDEEP
swift
Ben Packard
Ben Packard

Reputation: 26476

Mutating array extension vs inout function

Given an array of Color objects, I want to be able to promote a certain color to the top of the array, based on a condition. Let's assume each Color has a boolean isBright property, and the test for promoting a color will be $0.isBright. I want to promote the first such color.

I would to provide a more general way to promote colors based on different tests. I have successfully accomplished this via two approaches, but I am not sure which is better. Please forgive me if I name or describe these techniques incorrectly.

Option A - inout Function

private func promoteColor(from colors: inout [Color], where promote: ((Color) -> Bool)) {
    if let index = colors.index(where: promote) {
        let color = colors.remove(at: index)
        colors.insert(color, at: 0)
    }
}

// call site:
promoteColor(from: &colors, where: { $0.isBright })

Option B - Mutating Array Extension

extension Array where Element: Color {
    mutating func promote(where promote: (Color) -> Bool) {
        if let index = index(where: promote) {
            let color = remove(at: index)
            insert(color, at: 0)
        }
    }
}

// call site:
colors.promote(where: { $0.isBright })

I prefer the readability of option B, but I'm not sure what else I should be considering when deciding which one to use. Are there any advantages or disadvantages of either?

Edit

I suppose Option B can be generalized:

extension Array {
    mutating func promote(where promote: (Element) -> Bool) {
        if let index = index(where: promote) {
            let element = remove(at: index)
            insert(element, at: 0)
        }
    }
}

Upvotes: 1

Views: 261

Answers (1)

bscothern
bscothern

Reputation: 2014

inout should be avoided when possible especially when you have mutating funcs.

Your best easy option is probably this:

extension Array {
    mutating func promote(where predicate: (Element) -> Bool) {
        if let index = index(where: predicate) {
            let element = remove(at: index)
            insert(element, at: 0)
        }
    }
}

Here is one other little update I thought of last night. If you are want more performance.

This second version will avoid the extra moving of elements after the promoted element. I've also tested it where the promoted element is the first element and it is safe with this logic.

extension Array {
    mutating func promote(where predicate: (Element) -> Bool) {
        if let index = index(where: predicate) {
            withUnsafeMutableBufferPointer { (bufferPointer: inout UnsafeMutableBufferPointer<Element>) -> Void in
                let promotedElement = bufferPointer[index]
                for i in (0..<index).reversed() {
                    bufferPointer[i + 1] = bufferPointer[i]
                }
                bufferPointer[0] = promotedElement
            }
        }
    }
}

Upvotes: 4

Related Questions