Asynchronous call in for loop fires two completions

Question

I have a method to retrieve data. This data can but doesn't have to contain an image. The method below is retrieving the data in order which is crucial for the app:

    static func getWishes(dataSourceArray: [Wishlist], completion: @escaping (_ success: Bool, _ dataArray: [Wishlist]) -> Void){
    
    var dataSourceArrayWithWishes = dataSourceArray
    
    let db = Firestore.firestore()
    let userID = Auth.auth().currentUser!.uid
    var j = 0
    let dispatchSemaphore = DispatchSemaphore(value: 0)
    let dispatchSemaphoreOuter = DispatchSemaphore(value: 0)
    let dispatchQueue1 = DispatchQueue(label: "taskQueue")
    var listCounter = 0
    dispatchQueue1.async {
        for list in dataSourceArray {
            listCounter += 1
            db.collection("users").document(userID).collection("wishlists").document(list.name).collection("wünsche").order(by: "wishCounter").getDocuments() { ( querySnapshot, error) in
                
                if let error = error {
                    print(error.localizedDescription)
                    
                } else {
                    // dispatch group to make sure completion only fires when for loop is finished
                    // append every Wish to array at wishIDX
                    var i = 0
                    let dispatchQueue = DispatchQueue(label: "taskQueue1")
                    if (querySnapshot?.documents.count)! > 0 {

                        dispatchQueue.async {
                            for document in querySnapshot!.documents {
                                
                                let documentData = document.data()
                                let imageUrlString = document["imageUrl"] as? String ?? ""              
                                if let imageUrl = URL(string: imageUrlString) {
                                    KingfisherManager.shared.retrieveImage(with: imageUrl, options: nil, progressBlock: nil, completionHandler: { result in
                                        
                                        var image = UIImage()
                                        
                                        switch result {
                                        case .success(let abc):
                                            image = abc.image
                                            
                                        case .failure(let error):
                                            print(error)
                                            break
                                        }
                                        dataSourceArrayWithWishes[wishIDX].wishes.append(Wish(image: image))

                                        i = i + 1
                                        
                                        if i == querySnapshot?.documents.count {
                                            j = j + 1
                                            dispatchSemaphoreOuter.signal()
                                            dispatchSemaphore.signal()
                                            
                                            if j == dataSourceArray.count {
                                                print("completion1")
                                                completion(true, dataSourceArrayWithWishes)
                                            }
                                        } else {
                                            dispatchSemaphore.signal()
                                        }
                                    })
                                } else {
                                    dataSourceArrayWithWishes[wishIDX].wishes.append(Wish(image: image))
                                    
                                    i = i + 1
                                    
                                    if i == querySnapshot?.documents.count {
                                        j = j + 1
                                        dispatchSemaphoreOuter.signal()
                                        dispatchSemaphore.signal()
                                        
                                        if j == dataSourceArray.count {
                                            print("completion3")
                                            completion(true, dataSourceArrayWithWishes)
                                        }
                                    } else {
                                        dispatchSemaphore.signal()
                                    }
                                }
                                dispatchSemaphore.wait()
                            }
                        }
                    } else {
                        j = j + 1
                        dispatchSemaphoreOuter.signal()
                    }
                }
            }
            // without this code the function will not fire a completion if only a single empty list is in dataSourceArray
//                if listCounter == dataSourceArray.count {
//                    print("completion")
//                    completion(true, dataSourceArrayWithWishes)
//                }
//                continue
            }
        }
    }

This method works fine if if (querySnapshot?.documents.count)! > 0 is true but if a dataSourceArray only contains an empty list it fails because it never fires the completion. I tried fixing that with a listCounter and with the code at the bottom. This sort of works but is not clean because that always fires 2 completions which looks weird in the app. Anyone have an idea on how I could fix the issue if there is only an empty list in dataSourceArray?

I am really stuck here so I am happy for every help!

One thing you can definitely do is to extract parts of your code to smaller functions. This way your code will be clearer, more readable and you may even find the problem yourself. — pawello2222, Jul 31 '20 at 12:28
@pawello2222 thought about that as well but I couldnt come up with anything... — Chris, Jul 31 '20 at 12:30
@Chris but breaking apart your code to smaller functions can show you the direction where to dig in your problem. — Pete Streem, Jul 31 '20 at 12:32
@PeteStreem I am struggling to put the code in smaller functions because of all the `completion hanlding`.. — Chris, Aug 02 '20 at 21:53
Well I guess you should call your completion handler in the `else` branch of your `if (querySnapshot?.documents.count)! > 0` — ndreisg, Aug 05 '20 at 14:55
@ndreisg aaah your right! I meant to do that! To many `{}` -blocks :D thanks man — Chris, Aug 05 '20 at 15:00
That's exactly what pawello2222 and Pete Streem tried to point out ;) — ndreisg, Aug 05 '20 at 15:02
i know haha :D stupid beginners mistake... thanks for the help! Will try to make my functions smaller next time! — Chris, Aug 05 '20 at 15:03

score 1 · Accepted Answer · answered Aug 05 '20 at 15:06

You forgot to call your completion handler in the else branch of your if (querySnapshot?.documents.count)! > 0

Note: As other commenters already pointed out it would be good to split your code into multiple functions for better readability and maintainability.

You can pass on your completion handler to these functions as parameter.

Asynchronous call in for loop fires two completions

1 Answers1