udacity / ios-nd-networking

Resources for Udacity's iOS Networking with Swift course.
MIT License
172 stars 88 forks source link

Use [weak self] when loading image in detail view. #15

Open OwenLaRosa opened 5 years ago

OwenLaRosa commented 5 years ago

Remember how we accessed the global image view property in downloadPosterImage's completion handler?

TMDBClient.downloadPosterImage(path: posterPath) { data, error in
    guard let data = data else {
        return
    }
    let image = UIImage(data: data)
    self.imageView.image = image
}

When reviewing the code, one of our iOS Engineers at Udacity, @jsvatek, noted the following.

Are these strong selfs in your closures safe? The concern is creating a reference cycle with TMDBClient. I personally tend to over use [weak self]. At the cost of a little noise, it means readers don't need to reason about retain cycles, and prevents you from accidentally creating one in the future.

While the specific discussion of memory management issues on iOS is more than can be explained in a single GitHub issue, you may want to consider the advice that it's better "safe than sorry" when writing code. Because we're accessing self.imageView, which belongs to the detail view controller, from the completion handler, we do indeed need [weak self] here. This ensures the image view is not retained after the detail view (and therefore the image view) no longer exists.

What is [weak self]? How does one use it? A good discussion can be found here, and this is definitely something to pay attention to in the future. https://blog.bobthedeveloper.io/swift-retention-cycle-in-closures-and-delegate-836c469ef128

And you may also find Apple's discussion of Automatic Reference Counting worth a read to avoid memory issues in the future. https://docs.swift.org/swift-book/LanguageGuide/AutomaticReferenceCounting.html

AshishMK commented 5 years ago

just also come across too know about [unowned self] but [weak self] is better to use in Asynchronous call and closures TMDBClient.downloadPosterImage(posterPath: movie.posterPath!, completion: {[weak self] (data,error) in guard let data = data else{ return }

         self?.imageView.image = UIImage(data: data)
    })
masture commented 5 years ago

I am passing a function handleDownloadPosterImage(data:error) instead of closure. Now my question is how do I make weak self in the function instead of closure? Do I have save issue or not?

AsmaHero commented 5 years ago

[weak self] when loading image in movie detail view controller. This ensures the image view no longer exists if we did not need it or call it in order to safe memory app . Am I right?

BrentMifsud commented 5 years ago

I am passing a function handleDownloadPosterImage(data:error) instead of closure. Now my question is how do I make weak self in the function instead of closure? Do I have save issue or not?

instead of referring to self.imageview in your function,

create a variable:

weak var movieDetailVC = self

movieDetailVC.imageView.image = UIImage(data: data)

you can also use the keyword unowned instead of weak.

Here is an overview of the differences: https://krakendev.io/blog/weak-and-unowned-references-in-swift

masture commented 5 years ago

Thanks @BrentMifsud for taking time and answering this. 🙏