2

I have several UIViewControllers which are added to a content view. After calling my remove function, I'm noticing that the child UIViewController's deinit function is not being called unless I explicitly set the value of the child UIViewController to nil.

Is that the proper behavior or am I doing something wrong?

func removeViewController(fromCell cell:UICollectionViewCell, at indexPath:IndexPath){
    guard let childViewController = currentViewControllers[indexPath] else { return  }
    print("remove view controller called")
    childViewController.willMove(toParent: nil)
    childViewController.view.removeFromSuperview()
    childViewController.removeFromParent()
    currentViewControllers[indexPath] = nil
    // recycledViewControllers.insert(childViewController)
} 
ZGski
  • 2,398
  • 1
  • 21
  • 34

2 Answers2

3

The problem is that you have two references to the child view controller:

  • The reference maintained automatically by the parent view controller (childViewControllers, or children in modern Swift)

  • An additional reference that you yourself have added (currentViewControllers).

You therefore have to let both of them go before the child view controller can go out of existence. That is what your code does:

childViewController.willMove(toParent: nil)
childViewController.view.removeFromSuperview()
childViewController.removeFromParent()
// now `childViewControllers` / `children` has let go

currentViewControllers[indexPath] = nil
// now `currentViewController` has let go

So you're not doing anything wrong, per se, except insofar as you've added this extra strong reference by having this currentViewControllers in the first place. But I can see why you've done that: you want a way to pair index paths with child view controllers.

So you can live with your approach just fine, or if you really want to you can rig currentViewControllers to have only weak references to its values (by using NSMapTable instead of a dictionary).

matt
  • 515,959
  • 87
  • 875
  • 1,141
  • Thank you so much that makes a lot of sense. I was trying to cache the child ViewController instances so they wouldn't have to be reinitialized but I see without being able to deallocate the resources used by the childViewControllers this just becomes more resource intensive than creating new instances each time. – TheRedCamaro3.0 3.0 Apr 29 '19 at 17:04
  • That is very interesting, thank you so much how would I create a dictionary with weak references to its values? – TheRedCamaro3.0 3.0 Apr 29 '19 at 17:05
  • Sorry, I wasn't finished typing yet! _Now_ I've finished; see my revised answer. But really there is nothing so horrible about what you're doing now. You've figured out how to do "manual" memory management and as long as you do it correctly you'll be fine. – matt Apr 29 '19 at 17:06
  • I can not thank you enough it works exactly as expected with `NSMapTable`. You hit the nail right on the head! – TheRedCamaro3.0 3.0 Apr 29 '19 at 17:27
-1

Apple's Documentation says:

Swift handles the memory management of instances through automatic reference counting (ARC)...

and

Deinitializers are called automatically, just before instance deallocation takes place. You are not allowed to call a deinitializer yourself.

This basically means that you can't call the deinit() method yourself and that things won't be deinitialized unless there are zero strong references to them.

ZGski
  • 2,398
  • 1
  • 21
  • 34
  • Thank you for your answer, how would you recommend going about deallocating the resources used by a childViewController which no longer being displayed? – TheRedCamaro3.0 3.0 Apr 29 '19 at 16:54
  • 3
    He isn't trying to call `deinit` so it's hard to see how this answers the question. – matt Apr 29 '19 at 16:57
  • @TheRedCamaro3.03.0 You could release them from memory by setting them to `nil`, but this usually shouldn't be required. It's hard to know exactly what's going on without seeing more of your code. – ZGski Apr 29 '19 at 17:00
  • @matt It answers the question by explaining why OP's code works the way it does wit sources to support the explanation, and that they aren't inherently doing anything wrong (like they asked). – ZGski Apr 29 '19 at 17:03
  • I regret downvoting your answer, because at the time I did it I didn't know I was going to submit an answer of my own. That was rude of me, and I apologize for it. But I don't take back what I said; this answer does not respond to what the OP actually asks. – matt Apr 29 '19 at 17:13