Closures vs Delegate pattern

Question

I'm working with network request classes and I'm concerned about crashes. For instance, working with closures is really easy as you pass a callback method to a function:

// some network client
func executeHttpRequest(#callback: (success: Bool) -> Void) {
    // http request

    callback(true)
}

// View Controller
func reload() {
    networkClient.executeHttpRequest() { (success) -> Void in
        self.myLabel.text = "it succeeded" // NOTE THIS CALL
    }
}

However, since the process that should execute the callback is async, when callbacks interact with container class element (in this case an UIKit class) it may be vulnerable to crashes in situations like

1. The user navigated to another View Controller while the async task was still executing
2. The user pressed the home button while the async task was still executing
3. Etc...

So, when the callback finally gets fired, self.myLabel.text might result in a crash, as the View Controller to whom self was refering could already be deallocated.

Up to this point. Am I right or do swift implement something internally so that this never happens?

If I am right, then here's when the delegate pattern comes in handy, as delegate variables are weak references, which means, they are not kept in memory if deallocated.

// some network client

// NOTE this variable is an OPTIONAL and it's also a WEAK REFERENCE
weak var delegate: NetworkClientDelegate?

func executeHttpRequest() {
    // http request

    if let delegate = self.delegate {
        delegate.callback(success: true)
    }
}

Note how self.delegate, since it is a weak reference, it will point to nil if the View Controller (who implements the NetworkClientDelegate protocol) gets deallocated, and the callback is not called in that case.

My question would be: do closures have anything special that makes them a good choice in scenarios similar to this one, rather than going back to delegate pattern? It would be good if examples of closures (that won't end up in crashes due to nil pointer) are provided. Thanks.

Answer 1

So, when the callback finally gets fired, self.myLabel.text might result in a crash, as the View Controller to whom self was referring could already be deallocated.

If self has been imported into the closure as a strong reference, it is guaranteed that self will not be deallocated up until the closure has been finished executing. That is, the view controller is still alive when the closure gets called - even if it's view is not visible at this time. Statement self.myLabel.text = "it succeeded" will be executed, but even the label will not be visible, it will not crash.

There is, though, a subtle issue which can lead to a crash under certain circumstances:

Suppose, the closure has the last and only strong reference to the view controller. The closure finishes, and subsequently gets deallocated, which also releases the last strong reference to the view controller. This inevitable will call the dealloc method of the view controller. The dealloc method will execute on the same thread where the closure will be executed. Now, that the view controller is a UIKit object, it MUST be guaranteed that all methods send to this object will be executed on the main thread. So, IFF dealloc will be actually executed on some other thread, your code may crash.

A suitable approach would require to "cancel" an asynchronous task whose result is no longer needed by the view controller when it is "closed". This, of course, requires that your "task" can be cancelled.

To alleviate some issues with your former approach, you might capture a weak reference of your view controller instead of a strong reference when defining the closure. This would not prevent the asynchronous task to run up to its completion, but in the completion handler you can check whether the view controller is still alive, and just bail out if it does not exists anymore.

And, if you need to "keep" an UIKit object in some closure which may execute on some arbitrary thread, take care that this might be the last strong reference, and ensure this last strong reference gets released on the main thread.

See also: Using weak self in dispatch_async function

Edit:

My question would be: do closures have anything special that makes them a good choice in scenarios similar to this one, rather than going back to delegate pattern?

I would say, closures are the "better" approach in many use-cases:

Delegates are more prone to issues like circular references than closures (since they are "owned" by an object, and this object might be captured as a variable in the delegate).

The classic use-case for closure as completion handlers also improves the "locality" of your code making it more comprehensible: you state what shall happen when a task finished right after the statement invoking the task - no matter how long that may take.

The huge advantage with closures versus regular "functions" is that a closure captures the whole "context" at the time when it is defined. That is, it can refer to variables and "import" them into the closure at the time when it is defined - and use it when it executes, no matter when this happens, and when the original "stack" at definition-time is gone already.