Swifterswift: UISegmentedControlExtensions return array of optional values

Created on 15 Feb 2017  路  3Comments  路  Source: SwifterSwift/SwifterSwift

public var segmentTitles: [String?]

public var segmentImages: [UIImage?]

The implementation currently returns an empty array in the case where no titles exist.
I think it would be better to either:

a) Return an optional Array
b) Return an Array of non-optional elements

This would also eliminate double optional values from using .first property on the array

let doubleOptional = UISegmentedControl().segmentTitles.first

I would also like to remove the use of the while loops from the implementation of these properties.

I think its "swiftier" to use a for in range loop
so we don't have to keep track of and increment our own index.

Currently with while loop

public var segmentTitles: [String?] {
        get {
            var titles: [String?] = []
            var i = 0
            while i < numberOfSegments {
                titles.append(titleForSegment(at: i))
                i += 1
            }
            return titles
        }
        set {
            removeAllSegments()
            for (index, title) in newValue.enumerated() {
                insertSegment(withTitle: title, at: index, animated: false)
            }
        }
    }

With for in range loop:

public var segmentTitles: [String?] {
        get {
            var titles: [String?] = []
            for i in 0..<numberOfSegments {
                titles.append(titleForSegment(at: i))
            }
            return titles
        }
        set {
            removeAllSegments()
            for (index, title) in newValue.enumerated() {
                insertSegment(withTitle: title, at: index, animated: false)
            }
        }
    }

If anyone has some thoughts on this let me know. I'd like to make any changes under the same commit for testing of this class.

All 3 comments

    public var segmentTitles: [String] {
        let range = 0..<numberOfSegments
        return range.flatMap { titleForSegment(at: $0) }
    }

Another thought if changing these properties to contain non-optional values is approved.

@SD10 I totally agree with you on this, I think second approach with flatMap is better, feel free to add them to a new pull request and I'll merge it

@omaralbeik Awesome, I'll update them in the morning when I also add testing for this class. Thanks!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

omaralbeik picture omaralbeik  路  5Comments

omaralbeik picture omaralbeik  路  3Comments

g001613001 picture g001613001  路  5Comments

pawurb picture pawurb  路  3Comments

omaralbeik picture omaralbeik  路  5Comments