3

I'm kind of new to programming in general, so I have this maybe simple question. Actually, writing helps me to identify the problem faster.

Anyway, I have an app with multiple asynchronous calls, they are nested like this:

InstagramUnoficialAPI.shared.getUserId(from: username, success: { (userId) in

    InstagramUnoficialAPI.shared.fetchRecentMedia(from: userId, success: { (data) in

        InstagramUnoficialAPI.shared.parseMediaJSON(from: data, success: { (media) in

                guard let items = media.items else { return }
                self.sortMediaToCategories(media: items, success: {

                    print("success")

          // Error Handlers

Looks horrible, but that's not the point. I will investigate the Promise Kit once I get this working.

I need the sortMediaToCategories to wait for completion and then reload my collection view. However, in the sortMediaToCategories I have another nested function, which is async too and has a for in loop. 

func sortMediaToCategories(media items: [StoryData.Items],
                           success: @escaping (() -> Swift.Void),
                           failure: @escaping (() -> Swift.Void)) {

    let group = DispatchGroup()
    group.enter()


    for item in items {


        if item.media_type == 1 {

            guard let url = URL(string: (item.image_versions2?.candidates?.first!.url)!) else {return}

            mediaToStorageDistribution(withImageUrl: url,
                                       videoUrl: nil,
                                       mediaType: .jpg,
                                       takenAt: item.taken_at,
                                       success: { group.notify(queue: .global(), execute: {

                                        self.collectionView.reloadData()
                                           group.leave()

                                       })  },
                                       failure: { print("error") })

 //....

I can't afford the collection view to reload every time obviously, so I need to wait for loop to finish and then reload.

I'm trying to use Dispatch Groups, but struggling with it. Could you please help me with this? Any simple examples and any advice will be very appreciated.

Max Kraev
  • 740
  • 12
  • 31
  • For this purpose, your best bet is PromiseKit as it does exactly what you are after. Another thing you can look into is semaphores but that will require some detailed knowledge of threads – Malik Apr 19 '18 at 05:30

3 Answers3

10

The problem you face is a common one: having multiple asynchronous tasks and wait until all are completed.

There are a few solutions. The most simple one is utilising DispatchGroup:

func loadUrls(urls: [URL], completion: @escaping ()->()) {
    let grp = DispatchGroup()

    urls.forEach { (url) in
        grp.enter()
        URLSession.shared.dataTask(with: url) { data, response, error in
            // handle error
            // handle response
            grp.leave()
        }.resume()
    }

    grp.notify(queue: DispatchQueue.main) {
        completion()
    }
}

The function loadUrls is asynchronous and expects an array of URLs as input and a completion handler that will be called when all tasks have been completed. This will be accomplished with the DispatchGroup as demonstrated.

The key is, to ensure that grp.enter() will be called before invoking a task and grp.leave is called when the task has been completed. enter and leave shall be balanced.

grp.notify finally registers a closure which will be called on the specified dispatch queue (here: main) when the DispatchGroup grp balances out (that is, its internal counter reaches zero).

There are a few caveats with this solution, though:

  • All tasks will be started nearly at the same time and run concurrently
  • Reporting the final result of all tasks via the completion handler is not shown here. Its implementation will require proper synchronisation.

For all of these caveats there are nice solutions which should be implemented utilising suitable third party libraries. For example, you can submit the tasks to some sort of "executer" which controls how many tasks run concurrently (match like OperationQueue and async Operations).

Many of the "Promise" or "Future" libraries simplify error handling and also help you to solve such problems with just one function call.

CouchDeveloper
  • 18,174
  • 3
  • 45
  • 67
1

You can reloadData when the last item calls the success block in this way.

let lastItemIndex = items.count - 1

for(index, item) in items.enumerated() {

if item.media_type == 1 {

  guard let url = URL(string: (item.image_versions2?.candidates?.first!.url)!) else {return}

  mediaToStorageDistribution(withImageUrl: url,
                             videoUrl: nil,
                             mediaType: .jpg,
                             takenAt: item.taken_at,
                             success: { 
                               if index == lastItemIndex {
                                  DispatchQueue.global().async {

                                    self.collectionView.reloadData()
                                  }         
                                } 
                             },
                             failure: { print("error") })
  }
Kamran
  • 14,987
  • 4
  • 33
  • 51
  • The logic is clear, but it produces some glitches in the interface, what could it be? I refreshed from a main thread btw – Max Kraev Apr 19 '18 at 06:27
  • Not sure what kind of glitches you are facing. I tried to provide the solution for reloading only once when all the items processed the data. – Kamran Apr 19 '18 at 06:36
  • 2
    The access to `index` is not synchronised (aka data race) and can cause undefined behaviour. Making it thread-safe is a bit cumbersome, so I would suggest another solution. – CouchDeveloper Apr 19 '18 at 07:00
  • @CouchDeveloper isn't success block capturing its own index? – Kamran Apr 19 '18 at 07:01
  • There is no guarantee the last items network call will finish last. This wont always work. – pkorosec Apr 19 '18 at 07:05
  • @Kamran Variables (in contrast to `let` constants) will be capture by default as a _reference_. The actual value will be evaluated when the closure will be executed. You can't use a constant as index, as it would be evaluated at the time the closure is instantiated (only once) and wouldn't change. So, you have potentially multiple threads that read and modify the variable `index`. – CouchDeveloper Apr 19 '18 at 07:05
1

You have to move the group.enter() call inside your loop. Calls to enter and leave have to be balanced. If your callbacks of the mediaToStorageDistribution function for success and failure are exclusive you also need to leave the group on failure. When all blocks that called enter leave the group notify will be called. And you probably want to replace the return in your guard statement with a break, to just skip items with missing URLs. Right now you are returning from the whole sortMediaToCatgories function.

func sortMediaToCategories(media items: [StoryData.Items], success: @escaping (() -> Void), failure: @escaping (() -> Void)) {

    let group = DispatchGroup()

    for item in items {
        if item.media_type == 1 {
            guard let url = URL(string: (item.image_versions2?.candidates?.first!.url)!) else { break }

            group.enter()
            mediaToStorageDistribution(withImageUrl: url,
                                       videoUrl: nil,
                                       mediaType: .jpg,
                                       takenAt: item.taken_at,
                                       success: { group.leave() },
                                       failure: {
                                          print("error")
                                          group.leave()
            })
        }
     }

    group.notify(queue: .main) {
        self.collectionView.reloadData()
    }
 }
pkorosec
  • 676
  • 8
  • 13