0

I´m trying to set up an iteration for downloading images. The whole process works, but taking a look in the console´s output, something seems to be wrong.

func download() {

    let logos = [Logos]()
    let group = DispatchGroup()

    logos.forEach { logo in

        print("enter")
        group.enter()

        if logo?.data == nil {

            let id = logo?.id as! String

            if let checkedUrl = URL(string: "http://www.apple.com/euro/ios/ios8/a/generic/images/\(id).png") {

                print(checkedUrl)

                LogoRequest.init().downloadImage(url: checkedUrl) { (data)  in

                    logo?.data = data
                    print("stored")

                    group.leave()
                    print("leave")
                }
            }
        }
    }

    print("loop finished")
}

Output:

enter
http://www.apple.com/euro/ios/ios8/a/generic/images/og.png
enter
http://www.apple.com/euro/ios/ios8/a/generic/images/eg.png
enter
http://www.apple.com/euro/ios/ios8/a/generic/images/sd.png
enter
http://www.apple.com/euro/ios/ios8/a/generic/images/hd.png
loop finished
stored
leave
stored
leave
stored
leave
stored
leave

It looks like the iteration does not care about entering and leaving the DispatchGroup() at all. The webrequests are fired almost at the same time. In my opinion the output should look like this:

enter
http://www.apple.com/euro/ios/ios8/a/generic/images/og.png
stored
leave
enter
http://www.apple.com/euro/ios/ios8/a/generic/images/eg.png
stored
leave
...
loop finished

Did I oversee something? Would be awesome to get some ideas.

Josch Hazard
  • 323
  • 3
  • 20
  • Is the only problem that the DispatchGroup is not behaving correctly? Or is there an operation issue also with how your web requests are processed? – AgnosticDev Jul 04 '17 at 18:21
  • It's because you're just printing "loop finished" when you're done submitting the requests. As J. Doe says, if you want to be notified when all of these dispatch groups are done, use `group.notify`. – Rob Jul 04 '17 at 18:26
  • Unrelated, but if either `logo?.data` was not `nil` or if the unwrapping of the URL failed, your `group.enter()` and `group.leave()` calls will be unbalanced, and your group will never finish. That's not the issue here, but you should be wary of this. You want to make sure that the `enter` and `leave` calls will always be balanced, one-for-one. – Rob Jul 04 '17 at 18:28
  • @ AgnosticDev: No there is not operating issue. The code works fine for up to 10 values. But running it with 100, the app crashes. So the requests are not executed asynchronously. – Josch Hazard Jul 04 '17 at 18:34
  • @ Rob: So you think I should add a second `group.leave()`? Like I edited in the code? – Josch Hazard Jul 04 '17 at 18:38
  • No, the revised code is worse, now (a) calling `leave` outside of the completion block; and (b) calling it twice, too. (By the way, don't change the code in your question like that, as you're invalidating answers predicated on a prior code sample.) If you want to add a code sample, do so, but don't edit existing code samples. The correct dispatch group fix is shown in J. Doe's answer. – Rob Jul 04 '17 at 18:40
  • The requests are, most likely, executed asynchronously (assuming that `downloadImage` runs asynchronously). Your "loop finished" is simply not waiting for those asynchronous tasks to finish. Regarding your crash, that's a completely different issue, possibly a result of running out of memory or, if you're just dispatching stuff to a GCD global queue, you could run out of worker threads. It's impossible to say without some clues as to what `downloadImage` is doing and without seeing the details about the crash. – Rob Jul 04 '17 at 18:41
  • @ Rob: If I understand you right. Even if everything is working as it should, the `"loop finished"` doesn´t have to wait for the asynchronous tasks to be finished? – Josch Hazard Jul 04 '17 at 18:53
  • @ Rob: But still, if I´m using J. Does´s example, the second half of my output is `stored leave, stored leave...` Shouldn´t it be `enter stored leave, enter stored leave...` ? – Josch Hazard Jul 04 '17 at 19:09

1 Answers1

3

What about this:

group.notify(queue: .main) {
print("loop finished")
}

Instead of your normal print.

edit:

func download() {

let logos = [Logos]()  // NSManagedObject
let group = DispatchGroup()

logos.forEach { logo in


    if logo?.data == nil {
        let id = logo?.id as! String
        if let checkedUrl = URL(string: "http://www.apple.com/euro/ios/ios8/a/generic/images/\(id).png") {

            print(checkedUrl)
             print("enter")
            group.enter()
            LogoRequest.init().downloadImage(url: checkedUrl) { (data)  in
                //this is async I think

                coin?.logo = data
                print("stored")

                group.leave()
                print("leave")
            }
        }
    }
}

group.notify(queue: .main) {
print("loop finished")
}
}
J. Doe
  • 12,159
  • 9
  • 60
  • 114
  • I just tried it, but it doesn´t change anything. This is after/outside the loop, but I need to pause the loop inside itself. – Josch Hazard Jul 04 '17 at 18:29
  • @ J. Doe: I just tried it. Unfortunatelly it doesn´t. – Josch Hazard Jul 04 '17 at 18:47
  • @ J. Doe: Still, the second half of my output is a long row of `stored leave, stored leave ` – Josch Hazard Jul 04 '17 at 19:05
  • @ J. Doe: At least it should be `enter stored leave, enter stored leave `, right? – Josch Hazard Jul 04 '17 at 19:06
  • @ J. Doe: So in the first part of my output, I´m getting all `enters` at once, in the second half i´m getting the `leaves`. Shouldn´t it be, that no new enter is executed as long as the last `leave` is executed? – Josch Hazard Jul 04 '17 at 19:15
  • @JoschHazard No, it is exactly the output you want. Normally, all the enters will come first and than the leaves. At the end, the print "loop finished". should show up. Leave() is called when the async operation is completed. Enter() will get called when the function on the main thread is called (which is a few ms). Since the async operation usually takes longer than the few ms, you first get all the enters, followed by the leaves and when there are no groups anymore, the notify function is called. – J. Doe Jul 04 '17 at 19:32
  • @JoschHazard You are confused by async and sync. With my method, all async downloads will be executed at the same time which causing the less time to complete everything. If you want an output which is Enter, Leave, Stored, Enter etc. you could modify this function but I think my approach is the one you want – J. Doe Jul 04 '17 at 19:35
  • @ J. Doe: I think I got it now. Somehow i thought, the DispatchGroup() will stop the function itself and make it go on when a certain task is finished. But in reality it´s like the DispatchGroup() takes over a certain part of the function and manages it one by one in the background. Now the `loop finished` shows up at the end of my output and the app doesn´t crash anymore. Thank you both! – Josch Hazard Jul 04 '17 at 19:39
  • @ J. Doe: `all async downloads will be executed at the same time ` You mean they are taken into a kind of "list", which the DispatchGroup() is taking care of. Then executed one by one? – Josch Hazard Jul 04 '17 at 19:44