CODEWITHSUNDEEP
iosswiftsyntax
user1951992
user1951992

Reputation:

Is this Swift syntax efficient?

Would this for loop fire each time the getter is accessed ? Or does Swift cache it 'behind the scenes' ?

var colors: [UIColor] {
get {
  var colors = [UIColor]()
  let palette = [UIColor.redColor(), UIColor.greenColor(), UIColor.blueColor(), UIColor.orangeColor(), UIColor.purpleColor(), UIColor.yellowColor()]
  var paletteIndex = 0
  for _ in 0..<photos.count {
    colors.append(palette[paletteIndex])
    paletteIndex = paletteIndex == (palette.count - 1) ? 0 : ++paletteIndex
  }
  return colors
}
}

In objective-c a getter like this would be positioned behind a check on the private ivar, so that the ivar is set once and then the ivar returned on subsequent calls.

Upvotes: 2

Views: 128

Answers (5)

Airspeed Velocity
Airspeed Velocity

Reputation: 40965

From a Swift logic perspective, it will run every time, but that said, there are some circumstances when the optimizer could turn the entire calculation into a constant, in which case it wouldn’t matter.

There are two things standing in the way of that: the palette array, and the photos.count property. In theory, the palette array never changes, but the compiler can’t know this because it doesn’t know that a function like redColor() always returns the same value. So hoist that array creation out of the property.

photos.count there is probably no fixing, assuming photos changes in size dynamically during the course of the program.

But this will give you the same effect and not require the creation of any arrays at all:

struct PaletteCycler {
    let palette = [
        UIColor.redColor(), UIColor.greenColor(),
        UIColor.blueColor(), UIColor.orangeColor(),
        UIColor.purpleColor(), UIColor.yellowColor(),
    ]

    subscript(idx: Int)->UIColor {
        return palette[idx%palette.count]
    }
}

let colors = PaletteCycler()

Since everything is constant there is very little runtime cost to fetching a color. Unlike the original version, this does not create an array every time. Since mod and fetch is pretty efficient, and the palette variable is constant, it should be fast.

By the way, if things weren’t so constant and you really did want an array, you could rewrite your loop using map like so:

let palette = [
    UIColor.redColor(), UIColor.greenColor(),
    UIColor.blueColor(), UIColor.orangeColor(),
    UIColor.purpleColor(), UIColor.yellowColor(),
]

var colors: [UIColor] = {

    // Swift 2.0 syntax. For 1.2, write
    // indices(photos).map
    return photos.indices.map {
        palette[$0 % palette.count]
    }

}

Upvotes: 0

Eiko
Eiko

Reputation: 25632

It fires everytime the getter is called. No way this could be optimized away.

You might want to keep a preset colors array in sync with the photos property, i.e. change it directly when setting the photos.

I also dislike the tight coupling in that place which seems unnecessary. I think it might be worthwhile to refactor this and just return a particular color for a given photo index (which is trivial using the modulo operator).

So my suggestion is to keep the palette as an instance variable, and just make your method return palette[index % palette.count], which will give the correct color immediately.

Upvotes: 2

user1951992
user1951992

Reputation:

I will be doing this, as @Eiko suggested

let palette = [UIColor.redColor(), UIColor.greenColor(), UIColor.blueColor(), UIColor.orangeColor(), UIColor.purpleColor(), UIColor.yellowColor()]

override func viewDidLoad() {
    super.viewDidLoad()
    ...
}

override func collectionView(collectionView: UICollectionView, cellForItemAtIndexPath indexPath: NSIndexPath) -> UICollectionViewCell {
    let cell = collectionView.dequeueReusableCellWithReuseIdentifier("AnnotatedPhotoCell", forIndexPath: indexPath) as UICollectionViewCell
    cell.contentView.backgroundColor = palette[indexPath.item % palette.count]
    return cell
}

Upvotes: 1

Kametrixom
Kametrixom

Reputation: 14973

You can use lazy initialisation:

lazy var colors : [UIColor] = {
    var colors = [UIColor.redColor]
    // blablabla
    return colors
}()

Which will be run just once when you try to access the colors the first time. However if the colors need to update throughout your class lifetime i suggest using a function to recapculate them:

func recalcColors() -> [UIColor] {
    var colors = [UIColor.redColor]
    // blablabla
    return colors
}

lazy var colors = recalcColors()

And when you need to update them you can call colors = recalcColors()

Upvotes: 0

dopcn
dopcn

Reputation: 4218

You are using computed property which do not actually store a value and the value of a computed named value or a computed property is not stored in memory. It's in the swift programming book

Upvotes: 0

Related Questions