Weak self in closures and consequences example

Question

I have done abit of research on stackoverflow and apple's documentation about ARC and Weak/Unowned self (Shall we always use [unowned self] inside closure in Swift). I get the basic idea about strong reference cycle and how it is not good as they cause memory leaks. However, I am trying to get my head around when to use Weak/Unowned self in closures. Rather then going into the "theory", I think it would really really help if someone could kindly explain them in terms of the bottom three cases that I have. My questions is

Is it OK to put weak self in all of them (I think for case two there is no need because I saw somewhere that UIView is not associated with self?. However, what if I put weak self there, is there anything that can cause me headache?
Say if the answer is No, you cant put weak self everywhere in all three cases, what would happen if I do (example response would be much appreciated...For example, the program will crash when this VC ....

This is how I am planning to use weakSelf Outside the closure, I put weak var weakSelf = self Then replace all self in closure with weakSelf? Is that OK to do?

Case 1:
FIRAuth.auth()?.signInWithCredential(credential, completion: {    (user: FIRUser?, error: NSError?) in
    self.activityIndicatorEnd()
    self.performSegueWithIdentifier(SEGUE_DISCOVER_VC, sender: self)
})

Case 2:
UIView.addKeyframeWithRelativeStartTime(0.0, relativeDuration: 0.1, animations: {
    self.messageLbl.alpha = 0.5
})

Case 3: 
//checkUserLoggedIn sends a request to firebase and waits for a response to see if the user is still authorised
checkUserLoggedIn { (success) in
    if success == false {
        // We should go back to login VC automatically

    } else {        
        self.discoverTableView.delegate = self
        self.discoverTableView.dataSource = self

        // Create dropdown menu
        let menuView = BTNavigationDropdownMenu(navigationController: self.navigationController, title: self.dropDownItems.first!, items: self.dropDownItems)

        menuView.didSelectItemAtIndexHandler = {[weak self] (indexPath: Int) -> () in
            if indexPath == 0 {
                self?.mode = .Closest
                self?.sortByDistance()

            } else if indexPath == 1 {
                self?.mode = .Popular
                self?.sortByPopularity()

            } else if indexPath == 2 {
                self?.mode = .MyPosts
                self?.loadMyPosts()

            } else {
                print("Shouldnt get here saoihasiof")
            }
        }

    // Xib
        let nib = UINib(nibName: "TableSectionHeader", bundle: nil)
        self.xibRef = nib.instantiateWithOwner(self, options: nil)[0] as? TableSectionHeader
        self.discoverTableView.registerNib(nib, forHeaderFooterViewReuseIdentifier: "TableSectionHeader")

        // Set location Manager data
        self.locationManager.delegate = self
        self.locationManager.desiredAccuracy = kCLLocationAccuracyBest

        // Check location service status
        if self.locationAuthStatus == CLAuthorizationStatus.AuthorizedWhenInUse {
            // Already authorised
            self.displayMessage.hidden = false

        } else if self.locationAuthStatus == CLAuthorizationStatus.NotDetermined {
            // Have not asked for location service before
            let storyboard = UIStoryboard(name: "Main", bundle: nil)
            let vc = storyboard.instantiateViewControllerWithIdentifier("LocationVC") as! LocationVC
            vc.locationVCDelegate = self
            self.presentViewController(vc, animated: true, completion: nil)

        } else {
            let alertController = UIAlertController(title: "Enable Location", message: "location is required to load nearby posts", preferredStyle: .Alert)
            let cancelAction = UIAlertAction(title: "Cancel", style: .Default, handler: nil)
            let settingsAction = UIAlertAction(title: "Settings", style: .Default, handler: { (action: UIAlertAction) in
                let settingsUrl = NSURL(string: UIApplicationOpenSettingsURLString)
                if let url = settingsUrl {
                    UIApplication.sharedApplication().openURL(url)
                }
            })
            alertController.addAction(settingsAction)
            alertController.addAction(cancelAction)
            self.presentViewController(alertController, animated: true, completion: nil)

            self.displayMessage.hidden = false
            self.displayMessage.text = "Could not determine your location to find nearby posts. Please enable location Service from settings"
        }

        // Styling
        self.refreshBtn.tintColor = COLOR_NAVIGATION_BUTTONS
        self.discoverTableView.backgroundColor = COLOR_DISCOVERVC_TABLEVIEW_BACKGROUND

        // Allow navigation bar to hide when scrolling down
        self.hidingNavBarManager = HidingNavigationBarManager(viewController: self, scrollView: self.discoverTableView)

        // Allow location to start updating as soon as we have permission
        self.locationManager.startUpdatingLocation()
   }
}

--Update-- Most of my code looks like case 3 where everything is wrapped inside a closure that either check if there is internet connectivity before any of the action is taken place. So I might have weak self everywhere??

--Update 2--

Case 4: 
// The haveInternetConnectivity function checks to see if we can reach google within 20 seconds and return true if we can 
haveInternetConnectivity { (success) in
    if success == false {
        self.dismissViewControllerAnimated()
    } else {        
        self.label.text = "You are logged in" 
        self.performSegueWithIdentifier("GoToNextVC")
    }
}

Question about case 4. Am I correct to say that even though this closure does not have weak/unowned self, it will never create strong reference (and memory leak) because even if the VC is dismissed before the completion block is executed, Xcode will try to run the code inside the completion block when we have confirmed internet status and will just do nothing (No crash) because self doesn't exist anymore. And once the code reached the last line inside the closure, the strong reference to self would be destroyed hence deallocate the VC?

So putting [weak Self] in that case would just mean that xcode would ignore those lines (as oppose to try and run it and nothing happens) which would mean a better practice but no issues on my hand either way

Rob · Accepted Answer · 2016-08-21T05:54:15.900

The question should not be "can I use weak reference," but rather "should I use weak reference." You use weak references to avoid strong reference cycles or to keep a closure from hanging on to something after it may have been disposed. But don't just add weak references because you can.

In case 1, you probably do want to use [weak self]. Why? Because if the view controller was dismissed while the authorization was underway, do you really want to keep a reference to a view controller that was dismissed? Probably not in this case.
In case 2, you theoretically could use [weak self] in the animation block, but why would you? There's no reason to. The weak reference is something you do with completion handlers and/or closure variables, but for an animation block it offers no utility, so I wouldn't do it there. To use weak here suggests a misunderstanding of the memory semantics involved.
In case 3, you have two separate issues.
- In the didSelectItemAtIndexHandler, that probably should use [unowned self] because the object's own closure is referring to itself.
  
  It may be a moot issue, as I don't see you actually using that BTNavigationDropdownMenu (perhaps that initializer is adding itself to the navigation controller, but that's not a well designed initializer if so, IMHO).
  
  But as a general concept, when an object has a handler closure that can only be called when the object is still around, but shouldn't, itself, cause the object to be retained, you'd use [unowned self].
- In the broader checkUserLoggedIn closure, the question is whether that's a completion handler. If so, you probably should use [weak self], because this could be initiated and be running by the time self is dismissed, and you don't want to have checkUserLoggedIn keep a reference to a view controller that was dismissed. But you wouldn't want to use [unowned self] because that would leave you with dangling pointers if it had been released by the time the closure runs.
  
  As an aside, you contemplate:
```
weak var weakSelf = self 
```
  That is a bit unswifty. You would use the [weak self] pattern at the start of the checkUserLoggedIn closure. If you have an example where you're tempted to use weak var weakSelf = ..., you should edit your question, including an example of where you want to use that pattern. But this is not one of those cases.

Could you please elaborate what you mean by "offers no utility". Also, is it fair to say that any closure that involves any sort waiting from an internet request always need a weak or unowned self? And is it correct that in almost all of those case it would be weak self ? — user172902, Aug 21 '16 at 07:39
Re weak references in `animation` closures not offering utility: The `animation` closure supplied to `animateWithDuration` is not a completion handler that is run later (though there is a separate `completion` closure, but that's a separate ball of wax altogether). It is called immediately, and the animation is initiated, but the closure is not keeping any strong references to `self` for the duration of the animation. — Rob, Aug 21 '16 at 08:50
Re network closures always need weak/unowned: No, this is certainly not the case. For example, you might do something in this closure that _must_ be run (e.g. update local db regarding completion of network request; save downloaded image to cache; etc.). Only use weak _if you don't need/want the code in the closure to keep reference to the object_ (e.g. you're merely updating UI, but not updating caches or databases or model structures). Don't just mindlessly use `weak`: Look at what's in the closure and decide if it should be (or must be) weak or not. — Rob, Aug 21 '16 at 08:50
Is it true that if the VC never gets dismissed, I do not need to concern about strong reference cycle? — user172902, Aug 21 '16 at 14:55
Putting what I have learned, Could you please take a look at my update with case 4 and the associated question about it. — user172902, Aug 21 '16 at 14:59
Case 4 certainly should use `[weak self]`, too. The "perform segue" doesn't make sense if the view controller you were at has, itself, been dismissed. — Rob, Aug 21 '16 at 15:35

Weak self in closures and consequences example

1 Answers1