Is this a best practice way to make concurrent API calls?-CodePudding

I'm learning about concurrent API calls, and I'm currently applying them to a project.

Based on my code below, is this a best practice way to make concurrent calls? (Note: I'm purposely not using Alamofire)

I'm making my concurrent API calls in fetchMealPlan() to create a mealPlan with multiple meals that have pre-loaded images. I'm using two types of API calls in fetchMealPlanBasedOn() and loadImageURL().

class MealPlanManager {
    
    static func fetchMealPlan(mealReqs: [MealReq], completion: @escaping (([MealInfo]) -> Void)) {
        
        let dispatchGroup = DispatchGroup()
        var mealPlan = [MealInfo]()
        
        DispatchQueue.global().async {
            
            mealReqs.forEach { req in
                dispatchGroup.enter()
                self.fetchMealBasedOn(req) { result in
                    var mealInfo = result
                    self.loadImageURL(for: mealInfo) { image in
                        mealInfo.image = image
                        mealPlan.append(mealInfo)
                        dispatchGroup.leave()
                    }
                }
            }
            
            dispatchGroup.notify(queue: .main) {
                completion(mealPlan)
            }
        }
    }
    
    static func fetchMealBasedOn(_ req: MealReq, completion: @escaping ((MealInfo) -> Void)) {
        print("fetching meal")
        
        // Validate appId and appKey
        guard let appId = Bundle.main.infoDictionary?["edamam_app_id"] as? String,
              let appKey = Bundle.main.infoDictionary?["edamam_app_key"] as? String else { return }
        
        // Create urlPrefix with queryItems
        var urlPrefix = URLComponents(string: "https://api.edamam.com/api/recipes/v2")!
        
        let queryItems = [
            URLQueryItem(name: "type", value: "public"),
            URLQueryItem(name: "q", value: req.type),
            URLQueryItem(name: "app_id", value: appId),
            URLQueryItem(name: "app_key", value: appKey),
            URLQueryItem(name: "mealType", value: req.type),
            URLQueryItem(name: "calories", value: req.macros.calories),
            URLQueryItem(name: "imageSize", value: "REGULAR"),
            URLQueryItem(name: "random", value: "\(req.random)"),
            URLQueryItem(name: "nutrients[CHOCDF]", value: req.macros.carbs),
            URLQueryItem(name: "nutrients[FAT]", value: req.macros.fat),
            URLQueryItem(name: "nutrients[PROCNT]", value: req.macros.protein),
            URLQueryItem(name: "field", value: "label"),
            URLQueryItem(name: "field", value: "image"),
            URLQueryItem(name: "field", value: "images"),
            URLQueryItem(name: "field", value: "url"),
            URLQueryItem(name: "field", value: "shareAs"),
            URLQueryItem(name: "field", value: "yield"),
            URLQueryItem(name: "field", value: "dietLabels"),
            URLQueryItem(name: "field", value: "healthLabels"),
            URLQueryItem(name: "field", value: "ingredientLines"),
            URLQueryItem(name: "field", value: "ingredients"),
            URLQueryItem(name: "field", value: "calories"),
            URLQueryItem(name: "field", value: req.type),
            URLQueryItem(name: "field", value: "totalNutrients")
        ]
        urlPrefix.queryItems = queryItems
        urlPrefix.percentEncodedQuery = urlPrefix.percentEncodedQuery?.replacingOccurrences(of: " ", with: "+")
        
        // Create urlRequest
        var urlRequest = URLRequest(url: urlPrefix.url!)
        urlRequest.httpMethod = "GET"
        
        // Create URLSession dataTask with urlRequest
        let task = URLSession.shared.dataTask(with: urlRequest) { data, response, error in
            guard let data = data, error == nil else {
                print("\(String(describing: error?.localizedDescription))")
                return
            }
            
            guard let response = response as? HTTPURLResponse, response.statusCode == 200 else {
                return
            }
            
            do {
                let result = try JSONDecoder().decode(MealData.self, from: data)
                self.convertToMealInfo(result: result, req: req) { mealInfo in
                    completion(mealInfo)
                }
            } catch {
                print("Can't decode JSON data \(error.localizedDescription)")
            }
        }
        
        task.resume()
    }
    
    static func loadImageURL(for meal: MealInfo, completion: @escaping ((UIImage) -> Void)) {
        print("fetching image")
        guard let imageURL = meal.imageURL, let url = URL(string: imageURL) else { return }
        
        let task = URLSession.shared.dataTask(with: url) { data, response, error in
            guard let data = data, error == nil else {
                print("Please check connection \(String(describing: error?.localizedDescription))")
                return
            }
            
            guard let response = response as? HTTPURLResponse, response.statusCode == 200 else {
                return
            }
            
            let result: UIImage
            result = UIImage(data: data) ?? Image.defaultMealImage!
            
            completion(result)
        }
        
        task.resume()
    }
}

CodePudding user response：

In your code there are some logical mistakes. You call dispatchGroup.enter() before making request - it's ok but you call - dispatchGroup.leave() only in successful completion But you code have so many places then function finish without completion call, int these cases your dispatchGroup.notify will never be called. This approach is not safety, you should call completion block in all cases, just pass error, or change completion argument to Result<MealInfo>

CodePudding user response：

Using DispatchGroup is the correct way for handling concurrent networking call.

About your code, there's a potential problem that if the API failed and you don't call the completion handler, then the DispatchGroup will be stuck and doesn't call notify. So, be careful when handling concurrent calls with DispatchGroup (set timeout or make sure enter and leave are called properly).

Beside, there're other options:

Use DispatchSemaphore to control number of concurrent calls https://developer.apple.com/documentation/dispatch/dispatchsemaphore
Use OperationQueue
On new Swift version (if your project is support iOS 13 and above), you can use new swift concurrency feature. Using this way, your code is more clear and readable than other above ways. https://docs.swift.org/swift-book/LanguageGuide/Concurrency.html