4

I've been stuck with this problem for some time now, and I ran out of useful google results.

I've got two requests, the first one retrieves some links from a server and in the other request I download the images from the links. After all the images finished downloading, I return the result to a view controller.

Int the code requestBanners() is called from a UIViewController, entering the dispatch group. In theory, before leaving the dispatch group storeImage() should be called for every url that I got from the server, and on every call it should enter and leave the dispatch group, adding the image to an array. Once finished downloading all the images, it leaves the dispatch group, and fires the notify() to execute setUpCollection() on the UIViewController.

Using print() as guidance to see what's really happening, I get:

I'm downloading the urls

Now I'm in the UIViewController (this print() is called in the implementation of setUpCollection())

I'm downloading an image

When it actually should be downloading the urls, then downloading the images and only then, returning to the UIViewController

I've used semaphores at first, but then I thought using a dispatch group would be a cleaner solution.

On both approaches, the empty bannersArr is returned before finishing downloading the images. Maybe that compact map is messing everything up.

Here's the code:

class BannersHelper {
    
    private var bannersArr: [UIImage] = []
    private let dispatchGroup = DispatchGroup()
    private var delegate: LoadBanners? = nil
    
    init(delegate: LoadBanners) {
        self.delegate = delegate
        
        dispatchGroup.notify(queue: .main) {
            self.delegate?.setUpCollection(errImg: self.bannersArr)
        }
    }
    
    func requestBanners() {
        print("I'm downloading the urls\n")
        self.dispatchGroup.enter()
        ServiceParametria.getBanners(handler: { response in
            guard let bannersResponse = response as? BannerModel else {
                return
            }
            
            let banners = bannersResponse.data as! [DataBannerModel]
            let _ = banners.compactMap({self.storeImage(model: $0)})
            self.dispatchGroup.leave()
        })
    }
    
    private func storeImage(model: DataBannerModel) {
        print("I'm downloading an image\n")
        self.dispatchGroup.enter()
        ServiceParametria.getImage(url: model.urlImagen!, handler: { result in
            let image = result as! UIImage
            self.bannersArr.append(image)
            self.dispatchGroup.leave()
        })
    }
}

Is there a better way to do this or am I just using the dispatch group wrong?

pkamb
  • 33,281
  • 23
  • 160
  • 191
Paula Chittaro
  • 181
  • 10
  • Is the goal to download all the images before proceeding, or to download all the image _in some specific order_ before proceeding? – matt Sep 15 '20 at 20:33
  • The goal is to start downloading the urls, once I got the urls, start downloading the image stored in that url and when all the images have been downloaded, only then I should return the image array. I'll edit the question maybe the problem is not so clear – Paula Chittaro Sep 15 '20 at 20:42
  • Then dispatch group is the way to go. You are using it wrong but it is the right mechanism. – matt Sep 15 '20 at 20:45
  • Does this answer your question? [Waiting for multiple asynchronous download tasks](https://stackoverflow.com/questions/32642782/waiting-for-multiple-asynchronous-download-tasks) – pkamb Sep 15 '20 at 20:48
  • I don't think so, the answer given in that post is somehow what I already got in my code and it's not working correctly. – Paula Chittaro Sep 15 '20 at 20:52
  • You are never calling `requestBanners()` in the code you've given. The `dispatchGroup.notify` thus executes immediately without ever fetching images or waiting for `enter/leave` calls. – pkamb Sep 15 '20 at 21:14
  • Paula, stackoverflow ("SO") algorithm "asked" me to give you feedback on the content of your question since you are a new user. You will get better answers quicker if you list the links on SO you have investigated along with an explanation of how those helped or were inapplicable. . If someone answers your question in a comment, ask them to post that as a response so that you can mark it as answering question. . Welcome to SO. I have learned a great deal from the contributors here. Regards. – PhillipOReilly Sep 15 '20 at 21:58
  • @pkamb I've updated the question explaining what should be doing – Paula Chittaro Sep 16 '20 at 13:31
  • 2
    Even if you call `requestBanners()` later from a VC, it's too late. The `dispatchGroup.notify` is going to fire in `init` with an empty array of banners. It won't re-fire later automatically... the notify block is called *once* as soon as the [Enter/Leave count == 0](https://stackoverflow.com/q/45610271/1265393)... which in this case is immediately in `init()`. You need to move the `notify()` to the end of the `requestBanners()` function. – pkamb Sep 16 '20 at 14:30
  • @pkamb thanks a lot! That solved the problem, it makes a lot of sense now that you pointed out, I thought the problem had to do with the incorrect use of the `enter()` and `leave()` functions of the dispatch group. Could you post this as an answer? – Paula Chittaro Sep 16 '20 at 15:51
  • I'd vote to close this as a Duplicate of the canonical Dispatch_Group Questions, so I'll decline on adding an Answer. But please write an Answer yourself with your findings... I'll give it an upvote. – pkamb Sep 16 '20 at 16:33

1 Answers1

7

As @pkamb pointed out in the comments the problem was that I was trying to fire the dispatchGroup.notify() inside the init() method.

At that time of the execution, as I didn't call requestBanners() yet, the DispatchGroup counter was equal to zero and that led to a an early call to the delegate?.setUpCollection() with an empty array.

Moving the dispatchGroup.notify() block at the end of the requestBanners() method solved the problem :)

Paula Chittaro
  • 181
  • 10