Is this a good way to display asynchronous data?

Question

I'm new to asynchronous coding and am wondering if the method I'm using to fetch and display data is considered correct in swift.

This method gets a list of objects from a user's section in the database, then fetches a picture for each item in the list. The way I'm telling if all images have been fetched is pushing them to an array as they arrive, then if that array's length is equal to the list of objects I reload the view. Here is my code:

var pets = [String]()
var imgs = [UIImage]()

override func viewDidAppear(_ animated: Bool) {
        imgsLoaded = 0
        imgs.removeAll(keepingCapacity: false)  // Clear images array
        pets.removeAll(keepingCapacity: false)  // Clear list of objects

        let userRef = FIRDatabase.database().reference().child("users").child((FIRAuth.auth()?.currentUser?.uid)!).child("pets")

        userRef.observeSingleEvent(of: .value, with: { snapshot in  // fetch list of objects from database
            if (snapshot.value as? Bool) != nil {                   // User has no pets added
                self.loadScrollView()                               // Reload view
            } else if let snap = snapshot.value as? NSDictionary {  // User has pets
                for value in snap {
                    self.pets.append(value.key as! String)          // Append object to list of objects
                }
                for i in 0..<self.pets.count {                      // For each item in the list, fetch its corresponding image
                    let imgRef = FIRStorage.storage().reference().child("profile_images").child(self.pets[i]+".png")
                    imgRef.data(withMaxSize: 15 * 1024 * 1024) { (data, error) -> Void in
                        if error != nil {
                            print(error)
                        }

                        // Create a UIImage, add it to the array
                        self.imgs.append(UIImage(data: data!)!)      // Push image to list of images
                        self.imgsLoaded+=1
                        if self.imgsLoaded == self.pets.count {      // If same number of images loaded, reload the view
                            self.loadScrollView()
                        }
                    }
                }
            }
        })
    }

As I said, I'm new to asynchronous coding and would like to hear what the proper way of doing what I'm attempting. One problem with my method is the images array might not align with the data array since an image can be fetched out of order. I'd love to learn the best way to do this so let me know please!

Edit: A way to make sure my data lines up with the corresponding images is to set a dictionary of sorts with the key being the data and the value being the image I guess.

If your images and data are not stored in the same record, then sure, they'll be in undermined order. — OneCricketeer, Jan 20 '17 at 21:37

Dallas Johnson · Accepted Answer · 2017-01-25T20:33:21.977

I would suggest using a DispatchGroup rather than keeping count of the number of items in 2 arrays. This would also allow to keep track of the progress across multiple threads with each block registered with enter() and leave() on the group. Then once all blocks have entered and left the notify block is called which could refresh your UI. Also using a dictionary with a place holder image in case the loading of one of the images fails. would be better than ignoring the failure case.

This may also be easier to read and reason since there is no tracking the count of 2 arrays. The intention is clearer with enter() and leave()

var imgs = [String: UIImage]()
var dispatchGroup = DispatchGroup()

 override func viewDidAppear(_ animated: Bool) {

    let userRef = FIRDatabase.database().reference().child("users").child((FIRAuth.auth()?.currentUser?.uid)!).child("pets")

    userRef.observeSingleEvent(of: .value, with: { [weak self] snapshot in  // fetch list of objects from database
        if (snapshot.value as? Bool) != nil {                   // User has no pets added
            self?.loadScrollView()                               // Reload view
        } else if let snap = snapshot.value as? NSDictionary {  // User has pets
            for value in snap {
                self?.dispatchGroup.enter()
                let imgRef = FIRStorage.storage().reference().child("profile_images").child(self!.pets[i]+".png")
                imgRef.data(withMaxSize: 15 * 1024 * 1024) { (data, error) -> Void in
                    if let error = error {
                        print(error) // handle the error here but still call .leave() on the group to be sure the notify block is called.
                        imgs[value] = somePlaceHolderImage
                        self?.dispatchGroup.leave()
                    } else if let data = data, let image = UIImage(data: data) {
                        imgs[value] = image
                        self?.dispatchGroup.leave()
                    }

                    dispatchGroup.notify(queue: DispatchQueue.main, execute: {
                        self?.loadScrollView()
                    })
                }
            }
        }
    })
}

I've been reading up on GCD and trying out your solution but I'm still a bit confused, maybe you can clarify things for me. I had to add `self.` in front of the enter and leave dispatch group since those database requests are asynchronous. Because they're asynchronous, I'm wondering what the use of putting things inside a dispatch group is if their callbacks would be called after the request is fetched anyways. Can you help me understand why? Thanks — MarksCode, Jan 25 '17 at 11:39
The dispatch group takes into account the possibility of the callbacks being on different threads and by using the the `enter()` and `leave()` methods you can be sure that the until all are balanced out (i.e. all calls to `enter()` must have a corresponding call to `leave()`) the notify block will not get called too early. In your case you potentially have many simultaneous calls occurring in parallel that could complete at different times and you only want to `loadScrollView()` once all are completed. — Dallas Johnson, Jan 25 '17 at 20:27
I wrote the answer without compiling but the self would be warning about possible self retain issues of self within the block. to prevent this you should add the [weak self] to the blocks to prevent the call backs accidentally retaining self. I'll update the answer above to reflect that. — Dallas Johnson, Jan 25 '17 at 20:30
There are plenty of thorough explanations about the `[weak self]` but basically by capturing `self` as a weak local var in the closure if `self` was released (such as leaving the view controller before the process had completed) there would not a retain cycle generated between the call back and self causing a memory leak. I'm skimming over the idea but generally using weak self as above helps prevent this problem. — Dallas Johnson, Jan 25 '17 at 20:41
Got it. Another thing, how does `dispatchGroup.notify` know that all tasks have been completed? Is it because it's in a for loop and it knows how many `enters` and leaves` there should be? — MarksCode, Jan 25 '17 at 21:28

Is this a good way to display asynchronous data?

1 Answers1

Linked