In Swift, if I have a closure capturing [weak self], is it good practice to unwrap the optional self at the beginning of the closure?

user_942165 0
Viewed 468

I am using Swift for a macOS application, Xcode is 12.5.1. Imagine I have the following code:

func performAsyncTask(completion: { [weak self] (error: Error?) in 

     self?.someProperty = 0.0
     self?.someOtherProperty = 0.0
     // Other similar statements follow
})

Is it good choice to change it to:

func performAsyncTask(completion: { [weak self] (error: Error?) in 
     
     guard let self = self else { return }
     self.someProperty = 0.0
     self.someOtherProperty = 0.0
     // Other similar statements follow
})

I believe that the first implementation is better because self could become nil between the statements, so unwrapping at the beginning could be less clean. I hope an expert can bring me in the right direction. Thanks for your attention.

swift weak-references closures
2 Answers

I believe that the first implementation is better because self could become nil between the statements

And that's why the second implementation is in fact better! If self is not nil at the first statement, the first statement makes it so that self couldn't become nil between the statements. It retains self exactly for the remainder of the block. This is called "the weak–strong dance".

guard let self = self else { return }
      //  ^^^^ this self is _strong_, not weak; it retains self
user_3419940

I would argue the other way for the same reason that you give (in your first version self can be deallocated at any point while your closure code runs).

If the closure runs on a separate thread from the code that retains self, it is possible that the object will become nil between the different statements that reference self?. (This is unlikely, but possible.) If your closure code is doing something more complex than just assigning properties to self, having self get deallocated in the middle of it could render the whole closure pointless, or even make it fail. (Say you're doing image processing and the image goes away while doing it.)

In the second version, the guard let self = self statement creates a strong reference to self for the scope of the closure, meaning that the object referenced by self won't get deallocated until the closure completes.

I would rather know if self still exists at the beginning of the closure, and then run the closure code completely, than have to write my closure code defensively to keep checking "Has self been deallocated? How about now? Now? Ok, how about now?"

As a matter of coding style and readability, having to constantly deal with optional chaining/binding/nil coalescing is a pain, and makes your code harder to follow

user_2051850
Related