-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Comments
btw, is self cyclically retained here? I think also have a question: if let strongSelf = self {
} |
I don't think it matters that self is retained or not, because the closure is executed whenever the Although I'm not entirely sure what happens when you never call 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 |
By the way, the
|
Thanks, I updated the original code block to remove the |
I like it. It keeps the initialization of the thing close to the declaration of the thing. As a result, |
I like it too. 👍 |
If there are issues with a strong reference cycle then 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)? |
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? |
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 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 |
In the Apple docs that @sammyd links, Apple actually gives this as an example:
In the second link @sammyd gives (he's link happy today :] ), the author gives this as an example:
I don't think that using For simplicity, why not always use Regardless, I'm all for using this pattern. 👍 |
Note that the |
I like it, for both 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 |
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. 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. eg. Note - I've overriden the init method here - (no xib files)
From stackoverflow This class EventCell: UITableViewCell {
} VS
|
About The sample code given by Apple and linked above by @sammyd does no longer use 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 Now I don't know if |
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 eric.chamberlain@arctouch.com mailto:eric.chamberlain@arctouch.com Custom apps for world-class brands and the Fortune 500
|
@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 |
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 eric.chamberlain@arctouch.com mailto:eric.chamberlain@arctouch.com Custom apps for world-class brands and the Fortune 500
|
I don't follow you. None of those lazy vars requires lazy var d: String = { "\(self)" }()
lazy var asHTML: Void -> String = { "\(self)" } Which kind of lazy var does? |
Oops, calling asHTML indeed creates the retain cycle. Thanks @arctouch-ericchamberlain. I've been mistaken. |
The reason that you found the Apple docs don't use
They then go on to fix this with the addition of
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 |
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 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. |
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 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:
For UI components that are not allocated in a storyboard or unconditionally in I should mention that although it has not been accepted, there is a rather large change probably coming to how lazy works: What do you think about this: Lazy // 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
}
// :
} |
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. |
Just make your class(view controller?) conform to CLLocationManagerDelegate. |
@rayfix I've moved this snippet to super class. I missed to include CLLocationManagerDelegate protocol to base. Thank you 😀 |
As an FYI, Joe Groff of Swift has confirmed that lazy vars are automatically 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) |
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:
But it did not occur when I included it and made no other changes. |
@rondiel note that CLLocationManager's delegate declared as assign (iOS 10 SDK) |
A lot of us in tutorials these days are using patterns like this to initialize variables:
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?
The text was updated successfully, but these errors were encountered: