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

Extending UITableViewCell to preserve UIView background color when selecting the cell

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

Problem

When a UITableViewCell has subviews with background color set, when setSelected(_: animated:) gets called, all the views are made transparent.

This issue is also discussed here

I'm trying to solve this issue by extending UITableViewCell

extension UITableViewCell {
    private func backgroundColors(views: [UIView]) -> [UIColor?] {
        var colors = [UIColor?]()
        for view in views {
            colors.append(view.backgroundColor)
        }
        return colors
    }

    private func resetBackgroundColors(views: [UIView], colors: [UIColor?]) {
        for (index, view) in views.enumerate() {
            view.backgroundColor = colors[index]
        }
    }

    func preserveBackgroundColors(views: [UIView], @noescape f: () -> Void) {
        let colors = backgroundColors(views)
        f()
        resetBackgroundColors(views, colors: colors)
    }
}

class MyCell: UITableViewCell {
    override func setSelected(selected: Bool, animated: Bool) {
        preserveBackgroundColors([/* some views */]) {
            super.setSelected(selected, animated: animated)
            /* rest of method implementation */
        }
    }
}


I'm trying to avoid having this functionality in a class hierarchy.

Is this a good implementation?

Solution

This is a very strange approach for quite a few reasons.

First, the fact that this is an extension of UITableViewCell makes absolutely no sense to me. The method is completely procedural. It takes in an array of views (any views... from any where). From these views, it creates an array of colors to preserve the current background color. Then executes a function, f (which by the way, SwiftLint would never allow a single-letter variable name like this) (and you should definitely be using SwiftLint). And function f may or may not even change the colors of any of these views. And then we use parallel arrays, a very procedural approach, to reset these colors.

And would we even notice the color change at all?

Or is the intent here to hide the color change?

As for a more concrete review of some of the code you have, this function:

private func backgroundColors(views: [UIView]) -> [UIColor?] {
    var colors = [UIColor?]()
    for view in views {
        colors.append(view.backgroundColor)
    }
    return colors
}


Could be replaced with a single line of code.

let backgroundColors = views.map { $0.backgroundColor }


But... to make our code less procedural... why don't we build a tuple array:

let viewColorPairs = views.map { ($0, $0.backgroundColor) }


And then when it comes time to reset the background colors:

for (view, color) in viewColorPairs {
    view.backgroundColor = color
}


So now we've eliminated your first two functions, and your preserveBackgroundColors function is simplified into:

func preserveBackgroundColors(views: [UIView], @noescape closure: () -> Void) {
    let viewColorPairs = views.map { ($0, $0.backgroundColor) }
    closure()
    for (view, color) in viewColorPairs {
        view.backgroundColor = color
    }
}

Code Snippets

private func backgroundColors(views: [UIView]) -> [UIColor?] {
    var colors = [UIColor?]()
    for view in views {
        colors.append(view.backgroundColor)
    }
    return colors
}
let backgroundColors = views.map { $0.backgroundColor }
let viewColorPairs = views.map { ($0, $0.backgroundColor) }
for (view, color) in viewColorPairs {
    view.backgroundColor = color
}
func preserveBackgroundColors(views: [UIView], @noescape closure: () -> Void) {
    let viewColorPairs = views.map { ($0, $0.backgroundColor) }
    closure()
    for (view, color) in viewColorPairs {
        view.backgroundColor = color
    }
}

Context

StackExchange Code Review Q#124886, answer score: 2

Revisions (0)

No revisions yet.