Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazy initializing pattern #88

Closed
rwenderlich opened this issue Apr 15, 2015 · 29 comments
Closed

Lazy initializing pattern #88

rwenderlich opened this issue Apr 15, 2015 · 29 comments
Assignees

Comments

@rwenderlich
Copy link
Member

A lot of us in tutorials these days are using patterns like this to initialize variables:

lazy var locationManager: CLLocationManager = {
    let manager = CLLocationManager()
    manager.desiredAccuracy = kCLLocationAccuracyBest
    manager.delegate = self
    manager.requestAlwaysAuthorization()
    return manager
  }()

I've seen this for setting up controls like UILabels and such too. This has the advantage of keeping the initialization code in one handy place.

I thought I'd start a discussion to see if this is a pattern we want to encourage in the style guide?

@Igor-Palaguta
Copy link

btw, is self cyclically retained here? I think [unowned self] or [weak self] should be added

also have a question:
What is recommended name for strongSelf in this case, have not found this in your guide:

if let strongSelf = self {

}

@hollance
Copy link
Member

I don't think it matters that self is retained or not, because the closure is executed whenever the locationManager variable is used and then is destroyed.

Although I'm not entirely sure what happens when you never call locationManager. In that case, the closure may indeed be holding on to self. Interesting. :-)

Edit: Just tested this out and using self in this closure seems to be fine. I suppose the closure doesn't really get created until you call locationManager for the first time. And then it immediately ends and is destroyed again.

@hollance
Copy link
Member

By the way, the ! is not needed; this works just fine:

lazy var locationManager: CLLocationManager = {

@rwenderlich
Copy link
Member Author

Thanks, I updated the original code block to remove the !. But we still need to discuss the main question - should we encourage this pattern in our style guide or not?

@hollance
Copy link
Member

I like it. It keeps the initialization of the thing close to the declaration of the thing. As a result, viewDidLoad() tends to become shorter and easier to read.

@moayes
Copy link
Member

moayes commented Apr 16, 2015

I like it too. 👍

@sammyd
Copy link
Contributor

sammyd commented Apr 16, 2015

If there are issues with a strong reference cycle then [unowned self] should fix it I think:

lazy var locationManager: CLLocationManager = {
  [unowned self] in
  let manager = CLLocationManager()
  manager.delegate = self
  ...
  return manager
}()

I like this approach, but I'm struggling to see the objections. Is it just that this approach is somewhat non-obvious, and that it can have subtle nuances (such as the potential for strong retain cycles)?

(reference: https://developer.apple.com/library/prerelease/ios/documentation/Swift/Conceptual/Swift_Programming_Language/AutomaticReferenceCounting.html#//apple_ref/doc/uid/TP40014097-CH20-ID57)

@mbeaty
Copy link

mbeaty commented Apr 16, 2015

IMO this is just a pattern. Maybe it belongs in a swift-patterns repo. As for retain cycles, I'm missing it. It's just a typical delegate assignment. Why should this be treated any differently than if you just did the CLLocationManager initialization stuff in an initialization method? The delegate pattern typically encourages the delegate property (in this case, within CLLocationManager) to be defined as weak, no?

@sammyd
Copy link
Contributor

sammyd commented Apr 16, 2015

My mistake re strong retain cycles. It's not a problem because the closure is executed. If it were returning a closure then it might be necessary to use [unowned self], although not in this case because it's being assigned as a weakly-held delegate.

It's also worth bearing in mind that the closure specified to generate the lazy var should not depend on any changeable properties of the class. At the point that the lazy var is generated (through evaluation of the closure) it'll capture the current values of these dependencies. It'll be confusing if the var doesn't change.

See this example from http://mikebuss.com/2014/06/22/lazy-initialization-swift/

class Person {
    var name: String

    lazy var personalizedGreeting: String = {
        [unowned self] in
        return "Hello, \(self.name)!"
        }()

    init(name: String) {
        self.name = name
    }
}

I think this is confusing:

let p = Person(name: "sam")
p.personalizedGreeting
// => "Hello, sam!"

p.name = "definitely somebody else"
p.personalizedGreeting
// => "Hello, sam!"

After all this rambling, I still think it's a good pattern, and suitable for use in tutorials, but it's important that authors understand these issues so as not to make these relatively complex mistakes.

/i'll shush now

@JRG-Developer
Copy link
Member

In the Apple docs that @sammyd links, Apple actually gives this as an example:

lazy var asHTML: () -> String = {
    [unowned self] in
    if let text = self.text {
        return "<\(self.name)>\(text)</\(self.name)>"
    } else {
        return "<\(self.name) />"
    }
}

In the second link @sammyd gives (he's link happy today :] ), the author gives this as an example:

lazy var personalizedGreeting: String = {
        [unowned self] in
        return "Hello, \(self.name)!"
        }()

I don't think that using unowned self will ever be a problem in these cases... that is, in creating lazy properties... I imagine in certain cases, though (as @sammyd again mentions, returning a block, perhaps others?) not using unowned could be problematic.

For simplicity, why not always use unowned for lazy properties?

Regardless, I'm all for using this pattern. 👍

@hollance
Copy link
Member

Note that the asHTML variable stores the closure for later use. What we're talking about here is initializing a regular variable using a closure that is executed immediately.

@gregheo
Copy link
Contributor

gregheo commented May 20, 2015

I like it, for both lazy and non-lazy properties.

I'm not sure what the guidance would be in this guide though. Always do it that way? If you had some calculated constants or colors, I can also see the benefit of "the old way" initializing multiple things in viewDidLoad() to re-use objects. It feels like a case-by-case thing to me.

@johndpope
Copy link

I've used initWithFrame calls to instantiate ui views for years with subviews of uilabels / textviews / and it simplifies code and avoids tripping up on view life cycle calls - particularly when calling super classes.
Does it suffer a tiny performance hit? Maybe. Does this go against the flow of uikit and using nib files? Yes. Do I spend hours troubleshooting nib files? No. Do my apps ship on time? Yes. Am I worried? No. Are other developers appalled? probably. Do I care - No.
Simple code = Simple logic - Simple to maintain.

Some would argue that for performance or memory until the view controller is presented (it was pushed onto navigationcontroller) - then don't instantiate the controls (wait for viewDidLoad / loadView ). That was before we had access to lazy vars on view controllers themselves.
So this solves that problem of pre-emptive instantiation.

eg.
lazy var loginVC = { return LoginVC()}()

Note - I've overriden the init method here - (no xib files)

required public init() {
    super.init(nibName: nil, bundle: nil)
}

From stackoverflow
http://stackoverflow.com/questions/27585409/using-a-programmatically-created-uitableviewcell-subclass-in-swift/31801507#31801507

This

class EventCell: UITableViewCell {

var eventName: UILabel = UILabel()
var eventCity: UILabel = UILabel()
var eventTime: UILabel = UILabel()

override init(style: UITableViewCellStyle, reuseIdentifier: String?) {
    super.init(style: style, reuseIdentifier: reuseIdentifier)

    self.contentView.addSubview(eventName)
    self.contentView.addSubview(eventCity)
    self.contentView.addSubview(eventTime)
}

override func layoutSubviews() {
    super.layoutSubviews()

    eventName = UILabel(frame: CGRectMake(20, 10, self.bounds.size.width - 40, 25))
    eventCity = UILabel(frame: CGRectMake(0, 0, 0, 0))
    eventTime = UILabel(frame: CGRectMake(0, 0, 0, 0))

}

}

VS

class EventCell: UITableViewCell {
   lazy public  var lblName = {
    return UILabel (frame: CGRectMake(10, 0, self.bounds.size.width , 40))
  }()

  lazy public  var lblCity = {
   return UILabel (frame: CGRectMake(10, 40, self.bounds.size.width , 20))
  }()
 lazy public  var lblTime = {
      return UILabel (frame: CGRectMake(self.bounds.size.width - 80, 10, , 25))
   }()


   override init(style: UITableViewCellStyle, reuseIdentifier: String?) {
   super.init(style: style, reuseIdentifier: reuseIdentifier)

    self.contentView.addSubview(lblName)
    self.contentView.addSubview(lblCity)
    self.contentView.addSubview(lblTime)
 }

}  

@groue
Copy link

groue commented Dec 8, 2015

About [unowned self] used to prevent retain cycle: it is no longer the case.

The sample code given by Apple and linked above by @sammyd does no longer use [unowned self]:

lazy var asHTML: Void -> String = {
    if let text = self.text {
        return "<\(self.name)>\(text)</\(self.name)>"
    } else {
        return "<\(self.name) />"
    }
}

And indeed I could never miss a deinit by using a lazy var initialized with a closure that references a naked self.

Now I don't know if self is implicitly unowned or unowned(unsafe) in those closures. Yet we should stop making it explicit, since it is implicit.

@ghost
Copy link

ghost commented Dec 9, 2015

The asHTML example isn’t initializing a value with a closure, it is storing a closure in a variable. In the asHTML case, [unowned self] is still required to prevent a strong reference cycle. This is all mentioned in the linked Swift documentation.

Eric Chamberlain, Lead Architect - iOS
ArcTouch - App Development Studio

eric.chamberlain@arctouch.com mailto:eric.chamberlain@arctouch.com
+1-415-944-2000

Custom apps for world-class brands and the Fortune 500
arctouch.com/work | arctouch.com/blog

On Dec 8, 2015, at 8:18 AM, Gwendal Roué notifications@github.com wrote:

About [unowned self] used to prevent retain cycle: it is no longer the case.

The sample code https://developer.apple.com/library/prerelease/ios/documentation/Swift/Conceptual/Swift_Programming_Language/AutomaticReferenceCounting.html#//apple_ref/doc/uid/TP40014097-CH20-ID57 given by Apple and linked above by @sammyd https://github.com/sammyd does no longer use [unowned self]:

lazy var asHTML: Void -> String = {
if let text = self.text {
return "<(self.name)>(text)</(self.name)>"
} else {
return "<(self.name) />"
}
}
And indeed I could never miss a deinit by using a lazy var initialized with a closure that references a naked self.

Now I don't know if self is implicitly unowned or unowned(unsafe) in those closures. Yet we should stop making it explicit, since it is implicit.


Reply to this email directly or view it on GitHub #88 (comment).

@groue
Copy link

groue commented Dec 9, 2015

@arctouch-ericchamberlain, you're right, the sample code now stores a closure in the property.

Yet, I repeat myself, "I could never miss a deinit by using a lazy var initialized with a closure that references a naked self."

For exemple:

class C {
    lazy var d: String = { "\(self)" }()
    deinit { print("deinit") }
}

C()   // deinit
C().d // deinit

You can try it yourself. If you find a retain cycle, I'd be happy to use [unowned self] again.

@ghost
Copy link

ghost commented Dec 9, 2015

That’s because

lazy var something = {..}()

doesn’t retain anything. The closure is constructed and executed when the variable is initialized. Whereas,

lazy var something = {…}

creates a strong reference cycle when the variable is initialized.

The asHTML variable in the sample code falls into the latter case.

Remove () from your initializer, then see what happens.

Eric Chamberlain, Lead Architect - iOS
ArcTouch - App Development Studio

eric.chamberlain@arctouch.com mailto:eric.chamberlain@arctouch.com
+1-415-944-2000

Custom apps for world-class brands and the Fortune 500
arctouch.com/work | arctouch.com/blog

On Dec 9, 2015, at 2:10 PM, Gwendal Roué notifications@github.com wrote:

@arctouch-ericchamberlain https://github.com/arctouch-ericchamberlain, you're right, the sample code now stores a closure in the property.

Yet, I repeat myself, "I could never miss a deinit by using a lazy var initialized with a closure that references a naked self."

For exemple:

class C {
lazy var d: String = { "(self)" }()
deinit { print("deinit") }
}

C() // deinit
C().d // deinit
You can try it yourself. If you find a retain cycle, I'd be happy to use [unowned self] again.


Reply to this email directly or view it on GitHub #88 (comment).

@groue
Copy link

groue commented Dec 9, 2015

I don't follow you.

None of those lazy vars requires [unowned self]:

lazy var d: String = { "\(self)" }()
lazy var asHTML: Void -> String = { "\(self)" }

Which kind of lazy var does?

@groue
Copy link

groue commented Dec 9, 2015

Oops, calling asHTML indeed creates the retain cycle.

Thanks @arctouch-ericchamberlain. I've been mistaken.

@sammyd
Copy link
Contributor

sammyd commented Dec 10, 2015

@groue

The reason that you found the Apple docs don't use [unowned self] is because they're setting up the situation where there's a problem:

Unfortunately, the HTMLElement class, as written above, creates a strong reference cycle between an HTMLElement instance and the closure used for its default asHTML value.

They then go on to fix this with the addition of [unowned self]:

An unowned reference is the appropriate capture method to use to resolve the strong reference cycle in the HTMLElement example from earlier.

Which results in this implementation:

lazy var asHTML: Void -> String = {
    [unowned self] in
    if let text = self.text {
        return "<\(self.name)>\(text)</\(self.name)>"
    } else {
        return "<\(self.name) />"
    }
}

That documentation page has a good description of when retain cycles are possible and when you should use unowned or weak to break out of it.

@seivan
Copy link

seivan commented Jan 15, 2016

@johndpope

Try to compare with this implementation, not sure what I've missed but the samples I've seen from Apple, tend to avoid closure brackets for just setting stuff.

class EventCell: UITableViewCell {

    lazy public var lblName = UILabel(frame: CGRectZero)
    lazy public var lblCity = UILabel(frame: CGRect())
    lazy public var lblTime = UILabel(frame: CGRect(x: 0, y: 0, width: 0, height: 0))

} 

Disagree with the notion that lazy should be used for readability. It's only purpose is really performance, since it might introduce problems with state e.g. using mutable properties within the closure. You already have initialisation in one place, it's called designated init.

You're using lazy because you have something heavy that might not be executed thus not needed or you want to delay its execution which coincidentally allows the user to abort.

So chances are, the closure will retain self after your ownership of said class, creating a leak.
Always use weak/unowned even if you're 100% sure the closure will be called.
Eventually, I suspect that the compiler will handle that. A lazy property with a closure can only be executed once, think dispatch_once(..). Anyway, that's probably how I'll roll.

@rayfix
Copy link
Contributor

rayfix commented Apr 8, 2016

First, lets get the memory reference thing out of the way. (Even the top hit blog posts on lazy gets this wrong. We need to change that Maxime!) For reasons described above, this does not leak. That is: The closure is lazily executed once. If it is executed, self is retained and then released. If it is not executed it is never retained.

The below sample code definitively shows this:

import XCPlayground
XCPlaygroundPage.currentPage.needsIndefiniteExecution = true

class Person {
  init(name: String) {
    self.name = name
    print("\(name) is born.")
  }

  deinit {
    print("\(name) dies.")
  }

  var name: String

  lazy var greeting: String = {
    return "Hello \(self.name)!"
  }()

}

do {
  let t = Person(name: "Ray")
  print(t.greeting)
}

print ("Next")

// Prints: 
// Ray is born.
// Hello Ray!
// Ray dies.
// Next

do {
  let t = Person(name: "Ray")
}

print ("End of the world")

// Prints:
// Ray is born.
// Ray dies.
// End of the world

Next, lets look at this whole lazy thing. This is a great way to control initialization with regards to view controller as you pay exactly at the point of use. It is particularly important for CLLocationManager as you it is likely you want to control when access happens so it doesn't ask for permissions mid-segue.

Another pattern that is a little less javascript-y looking is to make a private method called and call that:

lazy var locationManager: CLLocationManager = self.makeLocationManager()

private func makeLocationManager() -> CLLocationManager
    let manager = CLLocationManager()
    manager.desiredAccuracy = kCLLocationAccuracyBest
    manager.delegate = self
    manager.requestAlwaysAuthorization()
    return manager
}

A couple of notes:

  • I am not sure which version of Swift this became possible.
  • No heap leak; same reasons above. Don't believe me? Try it!
  • locationManager has an explicit type annotation. This is currently required by the compiler or you get a cryptic error message about self being unresolved.
  • Works for classes but not immutable structs.

For UI components that are not allocated in a storyboard or unconditionally in viewDidLoad(), I think lazy is a good pattern. But I don't think I am ready to say "lazy all the things" as it can require some tricks such as LazyBoxes to get around immutability errors.

I should mention that although it has not been accepted, there is a rather large change probably coming to how lazy works:
https://github.com/apple/swift-evolution/blob/master/proposals/0030-property-behavior-decls.md

What do you think about this:

Lazy
Consider using lazy initialization for finer grain control over object lifetime.

// Mark: - Location services
extension MyViewController {
  lazy var locationManager: CLLocationManager = self.makeLocationManager()

  private func makeLocationManager() -> CLLocationManager
    let manager = CLLocationManager()
    manager.desiredAccuracy = kCLLocationAccuracyBest
    manager.delegate = self
    manager.requestAlwaysAuthorization()
    return manager
  }
  // :
}

@rayfix rayfix self-assigned this Apr 8, 2016
@rayfix rayfix added this to the Update April 2016 milestone Apr 8, 2016
@rayfix rayfix closed this as completed Apr 14, 2016
@AaronBratcher
Copy link

I don't mind the block style initialization of variables. In fact, by putting such initializations at the variable declaration allows for an easy Unit / UI testing override.

@muhasturk
Copy link

By the way define locationManager as class property is not working because of assigning manager.delegate = self

What do you suggest?

@rayfix
Copy link
Contributor

rayfix commented May 25, 2016

Just make your class(view controller?) conform to CLLocationManagerDelegate.

@muhasturk
Copy link

@rayfix I've moved this snippet to super class. I missed to include CLLocationManagerDelegate protocol to base.

Thank you 😀

@iwasrobbed
Copy link

As an FYI, Joe Groff of Swift has confirmed that lazy vars are automatically @noescape so they won't capture self.

https://twitter.com/jckarter/status/704100315587477504

Keep in mind you may still be able to capture self in other ways (e.g. block variables that capture self within a lazy var)

@rondiel
Copy link

rondiel commented Jul 16, 2016

Just wanted to let you know that using the new memory graph view in Xcode 8 I found that there was a capture when I did not use the unowned self construct in the following:

lazy var locationManager: CLLocationManager = {
    [unowned self] in
    var _locationManager = CLLocationManager()
    _locationManager.delegate = self
    _locationManager.desiredAccuracy = Set.shared.trackingAccuracy
    _locationManager.allowsBackgroundLocationUpdates = true
    _locationManager.activityType = .fitness      
    _locationManager.distanceFilter = Double(Set.shared.waypointInterval)
    return _locationManager
}()

But it did not occur when I included it and made no other changes.

@akkrat
Copy link

akkrat commented Nov 24, 2016

@rondiel note that CLLocationManager's delegate declared as assign (iOS 10 SDK)
@property(assign, nonatomic, nullable) id<CLLocationManagerDelegate> delegate;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests