is this a correct way to use Notification Center to execute tasks in a certain order avoiding memory leaks?

Question

I'm testing the usage of notifications and completion handlers to perform multiple time consuming tasks in a certain order. A colleague told me to check for possible memory leaks.

My questions are:

• my LongTasksClass: is this a proper way to use notifications and closures?
• is it correct to not remove observers after iOS 9?
• should be better make a singleton launched elsewhere?

in my view controller I have:

private var longTaskInstance: LongTasksClass? = LongTasksClass()

and in a button:

longTaskInstance?.startOperation()

my class

class LongTasksClass {
    
    enum TestingEnum: String {
        case firstOperation
        case secondOperation
        case stop
    }
    
    //public
    let testNotification = NSNotification.Name("test")
    
    //private
    private var myStatus: TestingEnum = .firstOperation
    private let notificationCenter = NotificationCenter.default
    
    
    init() {
        
        notificationCenter.addObserver(forName: self.testNotification, object: nil, queue: nil) { [weak self] notification in
            guard let self = self else {return}
            print(notification.userInfo?["info", default: "N/D"] ?? "no userInfo")
            
            self.doWholeOperation() {
                //first calls code in comletion
                //then calls this
                print("code after last completion")
            }
        }
        
    }
    
    
    func startOperation() {
        notificationCenter.post(name: self.testNotification, object: nil, userInfo: ["info" : "info start operation"])
    }
    
    //MARK: - private section -
    
    private func doWholeOperation(_ completion: @escaping (()) -> () ) {
        
        switch myStatus {
        case .firstOperation:
            print("very start: \(Date())\n")
            firstTimeConsumingTask { [weak self] in
                guard let self = self else {return}
                print("end first: \(Date())\n")
                self.myStatus = .secondOperation
                self.notificationCenter.post(name: self.testNotification, object: nil, userInfo: ["info" : "info sent from first task"])
            }
        case .secondOperation:
            secondTimeConsumingTask {
                print("end second: \(Date())\n")
                self.myStatus = .stop
                self.notificationCenter.post(name: self.testNotification, object: nil, userInfo: ["info" : "info sent from second task"])
            }
        case .stop:
            myStatus = .firstOperation
            let myCodeForCompletion: () = print("very end")
            completion(myCodeForCompletion)
            return
        }
    }
    
    private func firstTimeConsumingTask(_ completion: @escaping () -> ()) {
        print("start first task: \(Date())")
        //simulating long task
        DispatchQueue.main.asyncAfter(deadline: .now() + 3) {
            print("inside first task: \(Date())")
            completion()
        }
    }
    
    private func secondTimeConsumingTask(_ completion: @escaping () -> ()) {
        print("start second task: \(Date())")
        //simulating long task
        DispatchQueue.main.asyncAfter(deadline: .now() + 3) {
            print("inside second task: \(Date())")
            completion()
        }
    }
    
    //should not be required aymore
    deinit {
        print("‼️ called deinit")
        NotificationCenter.default.removeObserver(self)
    }
    
}

Answer 1

You must unregister notification observers created by addObserver(forName:object:queue:using). From the docs:

Return Value: An opaque object to act as the observer. Notification center strongly holds this return value until you remove the observer registration.

You must invoke removeObserver(_:) or removeObserver(_:name:object:) before the system deallocates any object that addObserver(forName:object:queue:using:) specifies.

This removeObserver is called on the returned value from addObserver (which you must store). It is not called on self. The point of the addObserver(forName:object:queue:using) is that it creates an opaque object observer; it does not make self an observer.

Regarding iOS 9, you're thinking of addObserver(_:selector:name:object:), which makes the first parameter (usually self) an observer, and generally does not require unregistering:

If your app targets iOS 9.0 and later or macOS 10.11 and later, you do not need to unregister an observer that you created with this function. If you forget or are unable to remove an observer, the system cleans up the next time it would have posted to it.

As always, consult the documentation.

Your deinit is incorrect (as you suspect). The call you make is not required, but you lack the required call to remove the observation you do make.

As a general rule, the selector-based API is much easier to use correctly. People tend to just have a fondness for block-based APIs even when they're less convenient.

In your example, the code required is:

class LongTasksClass {
...
    var observer: NSObjectProtocol
...
   init() {
        observer = notificationCenter.addObserver(forName: self.testNotification, object: nil, queue: nil) { [weak self] notification in
            ...   
        }     
    }
    ...
    deinit {
        notificationCenter.removeObserver(observer)
    }