swift - Dispatch group don't return fetched data

Question

I'm trying to use DispatchGroup for fetching data from multiple request. I cant understand why print(weatherData.fact.pressureMm!) is working, but data didn't appending inside dataArray and print(dataArray?[0].fact.pressureMm ?? "nil") print nil.

Also i'm try print data from complitionHandeler and result was same.

How i can append weatherData inside array and get value from complition correctly?

func fetchWeatherForCities (complitionHandeler: @escaping([YandexWeatherData]?)->Void) {
    var dataArray: [YandexWeatherData]?

    let group = DispatchGroup()

    for city in cities {
        group.enter()
        DispatchQueue.global().async {

            var urlString = self.urlString

            self.locationManager.getCoordinate(forCity: city) { (coordinate) in

                urlString += self.latitudeField + coordinate.latitude
                urlString += self.longitudeField + coordinate.longitude

                guard let url = URL(string: urlString) else {return}
                var request = URLRequest(url: url)
                request.addValue(self.apiKey, forHTTPHeaderField: self.apiField)


                let dataTask = URLSession.shared.dataTask(with: request) { (data, response, error) in
                    if let error = error {
                        print(error)
                    }

                    if let data = data {
                        guard let weatherData = self.parseJSON(withData: data) else {return}
                        print(weatherData.fact.pressureMm!)
                        dataArray?.append(weatherData)
                        print(dataArray?[0].fact.pressureMm ?? "nil")
                        group.leave()
                    }
                }
                dataTask.resume()
            }
        }
    }
    group.notify(queue: DispatchQueue.global()) {
        complitionHandeler(dataArray)
    }
}

Answer 1

A few issues:

1. You have paths of execution where, if an error occurred, you would not call leave. Make sure every path of execution, including every “early exit”, offsets the enter with a leave.

2. You defined dataArray to be an optional, but never initialize it. Thus it is nil. And dataArray?.append(weatherData) therefore will never append values.

Thus, perhaps:

func fetchWeatherForCities (completionHandler: @escaping ([YandexWeatherData]) -> Void) {
    var dataArray: [YandexWeatherData] = []
    let group = DispatchGroup()

    for city in cities {
        group.enter()

        var urlString = self.urlString

        self.locationManager.getCoordinate(forCity: city) { (coordinate) in
            urlString += self.latitudeField + coordinate.latitude
            urlString += self.longitudeField + coordinate.longitude

            guard let url = URL(string: urlString) else {
                group.leave()     // make sure to `leave` in early exit
                return
            }

            var request = URLRequest(url: url)
            request.addValue(self.apiKey, forHTTPHeaderField: self.apiField)

            let dataTask = URLSession.shared.dataTask(with: request) { data, response, error in
                guard
                    let data = data,
                    error == nil,
                    let weatherData = self.parseJSON(withData: data)
                else {
                    group.leave() // make sure to `leave` in early exit
                    print(error ?? "unknown error")
                    return
                }

                print(weatherData.fact.pressureMm!) // I'd advise against every doing force unwrapping on results from a third party service
                dataArray.append(weatherData)

                group.leave()
            }
            dataTask.resume()
        }
    }

    group.notify(queue: .main) {
        completionHandler(dataArray)
    }
}

As an aside, in the above, I have made two unrelated GCD changes, namely:

• Removed the dispatching of the network request to a global queue. Network requests are already asynchronous, so dispatching the creation of the request and the starting of that request is a bit redundant.

• In your notify block, you were using a global queue. You certainly can do that if you really need, but most likely you are going to be updating model objects (which requires synchronization if you're doing that from a background queue) and UI updates. Life is easier if you just dispatch that to the main queue.

---

FWIW, when you get past your current issue, you may want to consider two other things:

1. If retrieving details for many locations, you might want to constrain this to only run a certain number of requests at a time (and avoid timeouts on the latter ones). One way is to use a non-zero semaphore:

   DispatchQueue.global().async {
       let semaphore = DispatchSemaphore(value: 4)
   
       for i in ... {
           semaphore.wait()
   
           someAsynchronousProcess(...) {
               ...
   
               semaphore.signal()
           }
       }
   }
   

   If you have used semaphores in the past, this might feel backwards (waiting before signaling; lol), but the non-zero semaphore will let four of them start, and others will start as the prior four individually finish/signal.

   Also, because we are now waiting, we have to re-introduce the dispatch to a background queue to avoid blocking.

2. When running asynchronous requests concurrently, they may not finish in the order that you started them. If you want them in the same order, one solution is to store the results in a dictionary as they finish, and in the notify block, build a sorted array of the results:

   var results: [Int: Foo] = [:]
   
   // start all the requests, populating a dictionary with the results
   
   for (index, city) in cities.enumerated() {
       group.enter()
       someAsynchronousProcess { foo in
           results[i] = foo
           group.leave()
       }
   }
   
   // when all done, build an array in the desired order
   
   group.notify(queue: .main) {
       let array = self.cities.indices.map { results[$0] } // build sorted array of `[Foo?]`
       completionHandler(array)
   }
   

   That begs the question about how you want to handle errors, so you might make it an array of optionals (like shown below).

Pulling that together, perhaps:

func fetchWeatherForCities(completionHandler: @escaping ([YandexWeatherData?]) -> Void) {
    DispatchQueue.global().async {
        var results: [Int: YandexWeatherData] = [:]
        let semaphore = DispatchSemaphore(value: 4)
        let group = DispatchGroup()

        for (index, city) in self.cities.enumerated() {
            group.enter()
            semaphore.wait()

            var urlString = self.urlString

            self.locationManager.getCoordinate(forCity: city) { coordinate in
                urlString += self.latitudeField + coordinate.latitude
                urlString += self.longitudeField + coordinate.longitude

                guard let url = URL(string: urlString) else {
                    semaphore.signal()
                    group.leave()     // make sure to `leave` in early exit
                    return
                }

                var request = URLRequest(url: url)
                request.addValue(self.apiKey, forHTTPHeaderField: self.apiField)

                let dataTask = URLSession.shared.dataTask(with: request) { data, response, error in
                    defer {
                        semaphore.signal()
                        group.leave() // make sure to `leave`, whether successful or not
                    }

                    guard
                        let data = data,
                        error == nil,
                        let weatherData = self.parseJSON(withData: data)
                    else {
                        print(error ?? "unknown error")
                        return
                    }

                    results[index] = weatherData
                }
                dataTask.resume()
            }
        }

        group.notify(queue: .main) {
            let array = self.cities.indices.map { results[$0] } // build sorted array
            completionHandler(array)
        }
    }
}