Kenny Ho
Kenny Ho

Reputation: 459

Swift: DispatchGroup notify called before task finish

Why is dispatchGroup.notify being called before my stocks loop? Shouldn't it be called only after the task is done? Below is an image of my console to show you what I mean.

override func viewDidLoad() {
    super.viewDidLoad()

     fetch { (stocks) in
        self.hud.dismiss()

          APIManager.shareInstance.getStockList(for: self.fetchedStocks) { (result) in

            switch result {
            case .success(let stocks):
                DispatchQueue.main.async {
                    self.fetchedStocks = stocks
                    print(self.fetchedStocks)
                }

            case .failure(let error):
                print("Couldn't find Symbol")
                print(error.localizedDescription)

            }
        }
        DispatchQueue.main.async {
            print("Reload data")
            self.tableView.reloadData()
        }
    }

enter image description here

Upvotes: 1

Views: 773

Answers (1)

Rob
Rob

Reputation: 437592

You need to call dispatchGroup.enter() before getStockList, not inside the closure.

You also need to move the dispatchGroup.leave outside the stocks.forEach:

fetch { stocks in
    self.hud.dismiss()

    dispatchGroup.enter()

    APIManager.shareInstance.getStockList(for: self.fetchedStocks) { result in
        switch result {
        case .success(let stocks):
            stocks.forEach { stock in
                for index in 0..<stocks.count {
                    self.fetchedStocks[index] = stock
                    print("Stock Model:", self.fetchedStocks[index])
                }
            }

        case .failure(let error):
            print(error.localizedDescription)
        }

        dispatchGroup.leave()
    }

    dispatchGroup.notify(queue: .main) {
        print("Sucessfully retrieve necessary data, now reloading tableview")
        self.tableView.reloadData()
    }
}   

Alternatively, since it appears that getStockList is called only once, you can eliminate the dispatch group entirely:

fetch { stocks in
    self.hud.dismiss()

    APIManager.shareInstance.getStockList(for: self.fetchedStocks) { (result) in
        switch result {
        case .success(let stocks):
            stocks.forEach { (stock) in
                for index in 0..<stocks.count {
                    self.fetchedStocks[index] = stock
                    print("Stock Model:", self.fetchedStocks[index])
                }
            }

        case .failure(let error):
            print(error.localizedDescription)
        }

        DispatchQueue.main.async {
            print("Now reloading tableview")
            self.tableView.reloadData()
        }
    }
}

Note, the DispatchQueue.main.async isn’t needed if fetch calls this closure on the main queue already. And if it’s not calling it on the main queue, then you probably want to dispatch the updating of self.fetchedStocks to the main queue, as well, to ensure thread safety of this model object.


I’m not sure why you’re iterating through stocks. I would have thought that this would be sufficient:

fetch { stocks in
    self.hud.dismiss()

    APIManager.shareInstance.getStockList(for: self.fetchedStocks) { (result) in
        switch result {
        case .success(let stocks):
            DispatchQueue.main.async {
                self.fetchedStocks = stocks
                self.tableView.reloadData()
            }

        case .failure(let error):
            print(error.localizedDescription)
        }
    }
}

Again, that DispatchQueue.main.async isn’t needed if the fetch completion handler already runs on the main queue.

Upvotes: 1

Related Questions