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)
}
}
A few issues:
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 theenter
with aleave
.You defined
dataArray
to be an optional, but never initialize it. Thus it isnil
. AnddataArray?.append(weatherData)
therefore will never append values.Thus, perhaps:
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:
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:
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.
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: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: