1
votes

I have a UICollectionView in my class declared as @IBOutlet weak var artworkCollectionView: UICollectionView!

Inside this class there is one delegate method called by two other View Controllers, one of these VC is a pop up, the other one is a normal VC.

The delegate method gets some data from the database and then updates the collection view calling inside a closure: self.artworkCollectionView.reloadData()

When the delegate method is called by the pop up VC, then all works great. BUT when the delegate method is called by the normal VC when it gets to self.artworkCollectionView.reloadData() it gets the infamous Fatal error: Unexpectedly found nil while implicitly unwrapping an Optional value.

I have checked all the references to the cell reuseIdentifier and all is correct. I suspect that since the UICollectionView is declared as weak var, when I go from the current class to the pop up and then the pop up calls the delegate methods, the reference is not lost, but when I go from the current class to the normal VC and then the normal VC calls the delegate method the reference to my weak var is lost and so it is "seen" as nil.

@IBOutlet weak var artworkCollectionView: UICollectionView!

override func viewDidLoad() {
    super.viewDidLoad()
    // Set up
    artworkCollectionView.dataSource = self
    artworkCollectionView.delegate = self
    artworkCollectionView.isUserInteractionEnabled = true
    artworkCollectionView.allowsSelection = true
    artworkCollectionView.register(UINib(nibName: 
    "MyCollectionViewCell", bundle: nil), 
    forCellWithReuseIdentifier: "cell")
}


// delegate method
func reloadCollections() {

    retrieveAlbumRatings { (isAlbum) in
        if isAlbum {

            self.retrieveAlbumData(completion: { (isFinished) in

                if isFinished {
                    // Reload collection views
                    self.artworkCollectionView.reloadData()

                }
            })
        }
    }
}

If I am right, my question is: how can I give weak var artworkCollectionView: UICollectionView! a STRONG reference so that it does not get lost in the flow from the current class to the normal VC and back?

EDIT: here is what I tried so far:

  1. Remove “weak” from the outlet declaration so making it: @IBOutlet var artworkCollectionView: UICollectionView! But I got the same error

  2. I passed artworkCollectionView to the normal VC via override performSegue and then passed it back as an argument of the delegate method. This does not give me the fatal error but also it does not reload the UICollectionView because I think that anyway the weak reference to the UICollectionView outlet is lost.

Thanks for your help (disclaimer: I am pretty new to Swift..)

2
How do you access it from the delegate ? also which line gives the crash the reload ?Sh_Khan
Please show all relevant codes, your normal VC, delegate, etc. weak may not be the main reason.OOPer
From the delegate I access like this: self.delegate?.reloadCollections() The line that triggers the fatal error is self.artworkCollectionView.reloadData()TheGees
What happens if you delete "weak?" Ignoring possible reference cycle problems for now just to see if you get the same error.Ron
@Ron I have tried that, same problem (see the EDIT I have added with what I tried so far)TheGees

2 Answers

0
votes

Inside this class there is one delegate method called by two other View Controllers, one of these VC is a pop up, the other one is a normal VC.

The delegate method gets some data from the database and then updates the collection view calling inside a closure: self.artworkCollectionView.reloadData()

  1. The flow appears to be that you have a VC containing the code above, the VC can either open a pop-up or just do a standard push segue to the "normal VC".
  2. You want some operation to occur in either the pop-up VC or normal VC, load some data and then when the user is directed back to the originating VC, the UICollectionView is updated with that data.

Your problems are the following:

I passed artworkCollectionView to the normal VC via override performSegue and then passed it back as an argument of the delegate method. This does not give me the fatal error but also it does not reload the UICollectionView because I think that anyway the weak reference to the UICollectionView outlet is lost. You shouldn't be passing anything around like this in most cases unless you have a really good reason to do so (I don't see one).

You want a separation of concerns here. You have to think carefully about what you wanjt to pass between VCs to avoid making weird dependencies between them. I wouldn't pass outlets for multiple reasons, the first being that you now have to keep track of the outlet in multiple VCs if you ever decide to change it. The second being that it requires too much mental gymnastics to keep track of the state of the outlet since it's being passed around everywhere. The outlets are also only guaranteed to be set at certain phases of the lifecycle. For example if you retrieve the destination VC from the segue in prepareForSegue:sender: and attempt to reference the outlets at that time, they will all be nil because they haven't been set yet.

These are all good reasons why the VC that contains the code above should be the one (and the only one) maintaining control over what gets displayed in artworkCollectionView and when. The problem here is how you're approaching this, rather than having the pop-up or normal VC call the delegate method or doing weird things like passing outlets from one VC to another, just pass the data around instead.

The simplest example is:

  1. The pop-up VC and normal VC call some code to actually fetch the data.
  2. Then depending on how you actually segued to the pop-up VC or normal VC from original VC, use either parentViewController or presentingViewController to get the reference to the original VC.
  3. Set the data into the original VC through that reference.
  4. Dismiss the pop-up VC or normal VC if necessary (depends on your specific app, maybe you want the user to push a UIButton to dismiss instead of doing it for them).
  5. When the original VC comes back into view, add some code to a lifecycle method like viewWillAppear to have it load the contents of the data into the UICollectionView at that time.

I see no reason why you should be passing any outlets outside of the original VC that should be the one managing it.

0
votes

Short answer: Don't do that. You should treat a view controller's views as private. You should add a method to your view controller that other objects call to tell it to reload it's collection view.

The longer answer is that your view controller's collection view should stick around as long as the view controller is on-screen. It sounds like you don't have a very strong understanding of object lifecycle and how ARC works. You should read up on that and do some exercises until you understand it better.

Try something like this:

//Make artworkCollectionView a normal weak var, not implicitly unwrapped. 
//You'll need to change your other code to unwrap it every time you use it.

@IBOutlet weak var artworkCollectionView: UICollectionView?

...

func reloadCollections() {

    retrieveAlbumRatings { (isAlbum) in
        if isAlbum {
            //The construct `[weak self]` below is called a capture list
            self.retrieveAlbumData(completion: { [weak self] (isFinished) in
                guard let weakSelf = self else {
                    print("self is nil");
                    return
                }
            }
            if isFinished {
                // Reload collection views
                guard let collectionView = weakSelf.artworkCollectionView else {
                    print("collectionView is nil!")
                    return
                }
                collectionView.reloadData()
            })
        }
    }
}