2

I wonder how to eliminate of using self inside the DispatchQueue. As a good practice, we are supposed to use self only in the init()

func loadAllClasses() {
    DispatchQueue.global(qos: .background).async {
      self.classVM.fetchAllClasses(id: id, completion: { (classes, error) in
        DispatchQueue.main.async {
          if error != nil {
            self.showAlert(message: "try again", title: "Error")
          }
          if let classes = classes {
            self.classesList = classes
            self.classesCollectionView.reloadData()
          }
        }
      })
    }
  }
konya
  • 131
  • 2
  • 8

3 Answers3

11

Don't worry! DispatchQueue closures don't cause retain cycles.

vadian
  • 274,689
  • 30
  • 353
  • 361
  • Thanks for quick response. I wonder how to eliminate of using `self` inside the `DispatchQueue`. As a good practice, we are supposed to use `self` only in the `init()`. – konya Mar 09 '21 at 07:48
  • You cannot avoid `self` in the closure if you have to access a property on class level, but once again, there is no reason to worry about. – vadian Mar 09 '21 at 07:52
  • "Using `self` only in `init`" seems to be a very strange practice. To be honest, such a "programming rule" seems to me to lack some understanding of how instance methods work. I use `self` in almost any function, except the static ones. And using `self` does not mean that you have to write `self` explicitly. – Andreas Oetjen Mar 09 '21 at 08:11
  • 1
    "DispatchQueue closures don't cause retain cycles." this is a bit too generic ;) It depends how the closure is created - inline or explicit where a variable references the closure. – CouchDeveloper Mar 09 '21 at 09:12
4

You asked:

I wonder how to eliminate of using self inside the DispatchQueue.

In SE-0269, adopted in Swift 5.3, they introduced a pattern where if you include self in the capture list, it eliminates the need for all the self references in the closure:

func loadAllClasses() {
    DispatchQueue.global(qos: .background).async { [self] in  // note [self] capture list
        classVM.fetchAllClasses(id: id) { (classes, error) in
            DispatchQueue.main.async {
                if error != nil {
                    showAlert(message: "try again", title: "Error")
                }
                if let classes = classes {
                    classesList = classes
                    classesCollectionView.reloadData()
                }
            }
        }
    }
}

As an aside, fetchAllClasses would appear to already be asynchronous, so the dispatch to the global queue is unnecessary:

func loadAllClasses() {
    classVM.fetchAllClasses(id: id) { [self] (classes, error) in  // note [self] capture list
        DispatchQueue.main.async {
            if error != nil {
                showAlert(message: "try again", title: "Error")
            }
            if let classes = classes {
                classesList = classes
                classesCollectionView.reloadData()
            }
        }
    }
}

As a good practice, we are supposed to use self only in the init()?

No, you use self wherever it eliminates ambiguity or where you need to make potential strong reference cycles clear. Do not shy away from using self references.

But the intuition, to eliminate unnecessary self references is good, because where not needed, it ends up being syntactic noise. And the whole point of requiring self references inside a closure is undermined if your broader codebase sprinkles self references all over the place.


As an aside, above I illustrate that where you are willing to capture self, you can use [self] in syntax to make one’s code more succinct, eliminate lots of unnecessary self references inside the closure.

That having been said, we would generally want to use [weak self] reference in this case. Sure, as vadian suggested, there might not be any strong reference cycle risk. But it is a question of the desired lifecycle of the object in question. For example, let us assume for a second that fetchAllClasses could conceivably be very slow. Let us further assume that it is possible that the user might want to dismiss the view controller in question.

In that scenario, do you really want to keep the view controller alive for the sake of completing fetchAllClasses, whose sole purpose is to update a collection view that has been dismissed?!? Probably not.

Many of us would use [weak self] when calling a potentially slow, asynchronous process:

func loadAllClasses() {
    classVM.fetchAllClasses(id: id) { [weak self] (classes, error) in  // note `[weak self]` so we don't keep strong reference to VC longer than we need
        DispatchQueue.main.async {
            guard let self = self else { return }                      // if view controller has been dismissed, no further action is needed

            guard error == nil, let classes = classes else {           // handle error and unwrap classes in one step
                self.showAlert(message: "try again", title: "Error")
                return
            }

            self.classesList = classes                                 // otherwise, proceed as normal
            self.classesCollectionView.reloadData()
        }
    }
}

Yes, this has re-introduced the self references that we removed above, but this is a good pattern for asynchronous requests, in general: We never let some network request designed to update a view controller prevent that view controller from being deallocated when the user dismisses it.

A further refinement of this above pattern would be to design fetchAllClasses to be cancelable, and then in the view controller’s deinit, cancel any pending network requests, if any. That is beyond the scope of this question, but the idea is that, not only should we not retain the view controller longer than necessary, but we should cancel pending requests, too. This deinit cancelation pattern, though, only works if you used a weak reference in the closure.

Rob
  • 415,655
  • 72
  • 787
  • 1,044
3

"As a good practice, we are supposed to use self only in the init()"

This is certainly not true. In fact, you use self more often than you see it in code, since this is an implicit parameter set by the compiler when you use a member variable or member function. Basically, you would not be able to write any "real" object oriented code following this rule.

Also, using self in an init function is even special - since self is a reference to the value that is "under construction" and it is not even allowed to use self when the value is not yet completely initialised.

So, having said this, using self in a closure which will be dispatched is something completely different.

Capturing self strongly:

Given a member function where you use self in the closure as follows:

func loadAllClasses() {
    queue.async {
        self.foo = 
    }
}

Here, you capture a strong reference to self, and this will keep your object which is referenced by self alive for at least to the point where the closure will be called and finishes.

We may call this a "temporary" retain cycle - but assuming the completion handler will be called eventually the closure will be deallocated and with it the strong reference to self as well.

So, this resolves automatically - and we don't need to worry ..., well except we have made some programmer error where the completion handler will not be called (this is another chapter, though ;) )

Capturing self weakly:

You can prevent the closure keeping the object alive, if you want - by capturing self weakly. This is a decision you have to make, and you may want to keep the object alive in some cases and in other case you don't want the object being alive for up until the closure completes.

func loadAllClasses() { 
    queue.async { [weak self] in 
        self?.foo = 
    }
}

Here, the closure only has a weak reference to self and thus, it doesn't keep the object alive up to the point where it will be called. Within the closure you need to get a temporary strong reference to the weak self in order to use it. Note that it may be nil when the closure executes.

Using some other tricks:

Sometimes you need a certain value from a property of self - but you care less about self itself:

func loadAllClasses() { 
    let foo = self.foo
    queue.async { 
        let bar = foo
    }
}

Here, you completely avoid to capture self. You can use this, when you can say that all values are known when you create the closure and changes of their original value don't matter when you execute the closure later at some point in time.

CouchDeveloper
  • 18,174
  • 3
  • 45
  • 67