0

This is my working code to fetch one item from NASA API I used the completion handler as presented on Apple Programming book.

class PhotoInfoController {
    func fetchPhotoInfo(completion: @escaping (PhotoInfo?) -> Void) {
        let baseURL = URL(string: "https://api.nasa.gov/planetary/apod")!

    let query: [String:String] = [
        "api_key" : "DEMO_KEY"
    ]

    let url = baseURL.withQueries(query)!

    let task = URLSession.shared.dataTask(with: url) {
        (data, response, error) in
        let jsonDecoder = JSONDecoder()

        if let data = data,
           let photoInfo = try? jsonDecoder.decode(PhotoInfo.self, from: data) {
            completion(photoInfo)
        } else {
            print("Not found or data is not sanitazed.")
            completion(nil)
        }
    }
    task.resume()
   }
}

The problem I having a hard time to figuring it out is how you can return an array go items (PhotoInfo) via a completion handler. This is my code so far:

class PhotoInfoController {
    func fetchPhotoInfo(completion: @escaping ([PhotoInfo]?) -> Void) {
        let baseURL = URL(string: "https://api.nasa.gov/planetary/apod")!
    let currentDate = Date()
    let formatter = DateFormatter()
    formatter.dateFormat = "YYYY-MM-d"
    var photoInfoCollection: [PhotoInfo] = []
    
    for i in 0 ... 1 {
        let modifiedDate = Calendar.current.date(byAdding: .day,value: -i ,to: currentDate)!
        let stringDate = formatter.string(from: modifiedDate)
        let query: [String:String] = [
            "api_key" : "DEMO_KEY",
            "date" : stringDate
        ]
        
        let url = baseURL.withQueries(query)!
        let task = URLSession.shared.dataTask(with: url) { (data,
                                                            response,
                                                            error) in
            let jsonDecoder = JSONDecoder()
            
            if let data = data,
               let photoInfo = try? jsonDecoder.decode(PhotoInfo.self, from: data) {
                photoInfoCollection.append(photoInfo)
            } else {
                print("Data was not returned")
            }
        }
        task.resume()
    }
    
    
    completion(photoInfoCollection)
    }
}

Any ideas or guide will greatly appreciated Thanks!

Code Implemented after suggestions:

class PhotoInfoController {
    private let baseURL = URL(string: "https://api.nasa.gov/planetary/apod")!
    private let currentDate = Date()
    private let dateFormatter: DateFormatter = {
        let formatter = DateFormatter()
        formatter.dateFormat = "YYYY-MM-d"
        return formatter
    }()
    
    private let jsonDecoder = JSONDecoder()
    
    func fethPhotoInfo(itemsToFetch: Int, completion: @escaping ([PhotoInfo]?) -> Void) {
        var count = 0
        var photoInfoCollection: [PhotoInfo] = []
        
        for i in 0 ... itemsToFetch {
            let modifiedDate = Calendar.current.date(byAdding: .day, value: -i, to: currentDate)!
            
            let query: [String : String] = [
                "api_key" : "DEMO_KEY",
                "date" : dateFormatter.string(from: modifiedDate)
            ]
            
            let url = baseURL.withQueries(query)!
            let task = URLSession.shared.dataTask(with: url) {
                (data, response, error) in
                
                if let data = data,
                   let photoInfo = try? self.jsonDecoder.decode(PhotoInfo.self, from: data) {
                    photoInfoCollection.append(photoInfo)
                    count += 1
                    
                    if count == itemsToFetch {
                        completion(photoInfoCollection)
                    }
                    
                } else {
                    print("Data for \(self.dateFormatter.string(from: modifiedDate)) not made.")
                }
            }
            task.resume()
        }
    }
}
Jack1886
  • 37
  • 7
  • Whzt do you get instead expected result in photoInfoCollection ? – claude31 Jul 23 '20 at 19:21
  • photoInfoCollection gets filled correctly with NASA API Items, but when I unwrap the value on a tableview when is called I get 0 items. – Jack1886 Jul 23 '20 at 19:23
  • Screen Shot: https://imgur.com/a/QC4ipte – Jack1886 Jul 23 '20 at 19:30
  • So you check that the append occurs with the right data ? Most likely you have an async issue: you exit fetch before all fetch was executed. Could have a look at this tutorial: https://www.raywenderlich.com/5371-grand-central-dispatch-tutorial-for-swift-4-part-2-2 – claude31 Jul 23 '20 at 19:35
  • Does this answer your question? [Wait until swift for loop with asynchronous network requests finishes executing](https://stackoverflow.com/questions/35906568/wait-until-swift-for-loop-with-asynchronous-network-requests-finishes-executing) – zrzka Jul 23 '20 at 20:35
  • @L.Rivera the solution you've added to the question implements happy path only (if everything works). Imagine that one request (or more) fails, or if you wont be able to decode `PhotoInfo`, ... then your completion handler will never be called (`count == itemsToFetch` condition will never be `true`). Also the `counter` is not necessary, you can remove it and use just `photoInfoCollection.count == itemsToFetch`. – zrzka Jul 24 '20 at 07:17
  • @zrzka Sure, currently I am working on handling when something went wrong, hopefully NASA API provides the minimum date and I have it in a date picker. Basically if the internet never fails I will be able to obtain the data with no problem, also I will be implementing 'photoInfoCollection.count' is a nice tidy up. Thank you! – Jack1886 Jul 24 '20 at 18:29

2 Answers2

2

Your code won't work as written.

You use a for loop to start 2 URLSession dataTask objects. Those tasks are async; the code that invokes the network request returns right away, before the request has even been sent.

Then outside your for loop you invoke your completion handler, before your network requests have even had a chance to be sent out. You will need a mechanism to keep track of the number of pending requests and invoke the completion handler when both requests have finished.

Consider this function that simulates what you are doing:

func createAsyncArray(itemCount: Int, completion: @escaping ([Int]) -> Void) {
    var count = 0; //Keep track of the number of items we have created
    
    var array = [Int]()  //Create an empty results array
    
    //Loop itemCount times.
    for _ in 1...itemCount {
        let delay = Double.random(in: 0.5...1.0)
        
        //Delay a random time before creating a random number (to simulate an async network response)
        DispatchQueue.main.asyncAfter(deadline: .now() + delay) {
            
            //Add a random number 1...10 to the array
            array.append(Int.random(in: 1...10))
            
            //Increment the number of results we have added to the array
            count += 1
            print("In loop, count = \(count)")
            
            //If we have have added enough items to the array, invoke the completion handler.
            if count == itemCount {
                completion(array)
            }
        }
    }
    print("at this point in the code, count = \(count)")
}

You might call that code like this:

    let itemCount = Int.random(in: 2...10)
    print("\(itemCount) items")
    createAsyncArray(itemCount: itemCount) {   array in
        for i in 0..<itemCount {
            print("array[\(i)] = \(array[i])")
        }
    }

Sample output from that function might look like this:

9 items
at this point in the code, count = 0
In loop, count = 1
In loop, count = 2
In loop, count = 3
In loop, count = 4
In loop, count = 5
In loop, count = 6
In loop, count = 7
In loop, count = 8
In loop, count = 9
array[0] = 8
array[1] = 6
array[2] = 5
array[3] = 4
array[4] = 7
array[5] = 10
array[6] = 2
array[7] = 4
array[8] = 7

Note that the output displays "at this point in the code, count = 0" before any of the entries have been added to the array. That's because each call to DispatchQueue.main.asyncAfter() returns immediately, before the code inside the closure has been executed.

The function above uses a local variable count to keep track of how many items have been added to the array. Once the count reaches the desired number, the function invokes the completion handler.

You should use an approach like that in your code.

Edit:

You should be aware that your network requests may complete out of order. Your code submits itemsToFetch+1 different requests. You have no idea what order those requests will finish in, and it is very unlikely that the requests will complete in the order they are submitted. If your second request completes faster than the first, its closure will execute first.

Duncan C
  • 128,072
  • 22
  • 173
  • 272
  • Used part our your sample code to make mine work. Your solution of checking with a counter was the solution to call the completion handler inside my closure same I was doing previously on my working code. Thank You! – Jack1886 Jul 23 '20 at 23:46
  • Glad to help. Do you understand why your call to `completion(photoInfoCollection)` didn't work where you had it in your code? It's important that you study that until you really get it or async code will keep tripping you up again and again. – Duncan C Jul 24 '20 at 12:53
  • I think my error was that the completion handler is an important role to allow a function call and execute any code that is passed as a parameter to the function. Async code is used in the other hand to be run on main thread example: update UI. I am right? – Jack1886 Jul 24 '20 at 18:38
  • Async code is used for a variety of reasons. One of the more common is to handle long-running tasks without blocking the user interface. Network calls are a good example. – Duncan C Jul 24 '20 at 20:00
0

You're complicating it for yourself with trying to do everything in one method. Imagine you have the fetchPhotoInfo function working (it actually works, so, good job so far):

struct PhotoInfo: Codable {
    let copyright: String
}

class PhotoInfoController {
    private let base = "https://api.nasa.gov/planetary/apod"
    private let dateFormatter: DateFormatter = {
        let formatter = DateFormatter()
        formatter.dateFormat = "YYYY-MM-d"
        return formatter
    }()
    
    func fetchPhotoInfo(forDate date: Date, completion: @escaping (PhotoInfo?) -> Void) {
        guard var components = URLComponents(string: base) else {
            completion(nil)
            return
        }
        
        components.queryItems = [
            URLQueryItem(name: "api_key", value: "DEMO_KEY"),
            URLQueryItem(name: "date", value: dateFormatter.string(from: date))
        ]
        
        guard let url = components.url else {
            completion(nil)
            return
        }
        
        let task = URLSession.shared.dataTask(with: url) { (data, response, error) in
            guard let data = data else {
                completion(nil)
                return
            }
            
            let photoInfo = try? JSONDecoder().decode(PhotoInfo.self, from:data)
            completion(photoInfo)
        }
        task.resume()
    }
}

Your next goal is to fetch multiple photo info. Keep your class, keep your method and add another one which leverages what you already have. One way to achieve this is to use DispatchGroup:

func fetchPhotoInfo(forDates dates: [Date], completion: @escaping ([PhotoInfo]) -> Void) {
    // Group of tasks
    let taskGroup = DispatchGroup()
    
    // Result of photos
    var result: [PhotoInfo] = []
    
    // For each date ...
    dates.forEach {
        // ... enter the group
        taskGroup.enter()
        
        // Fetch photo info
        fetchPhotoInfo(forDate: $0) { photoInfo in
            defer {
                // Whenever the fetchPhotoInfo completion closure ends, leave
                // the task group, but no sooner.
                taskGroup.leave()
            }
            
            // If we've got the photo ...
            if let photoInfo = photoInfo {
                // ... add it to the result. We can safely add it here, because
                // the fetchPhotoInfo completion block is called on the
                // URLSession.shared.delegateQueue which has maxConcurrentOperationCount
                // set to 1 by default. But you should be aware of this,
                // do not rely on it and introduce some kind of synchronization.
                result.append(photoInfo)
            }
        }
    }
    
    // At this point, we told the URLSession (via fetchPhotoInfo) that we'd like to
    // execute data tasks. These tasks already started (or not, we don't know), but
    // they didn't finish yet (most likely not, but we don't know either). That's
    // the reason for our DispatchGroup. We have to wait for completion of all
    // our asynchronous tasks.
    taskGroup.notify(queue: .main) {
        completion(result)
    }
}

You can use it in this way:

let pic = PhotoInfoController()
pic.fetchPhotoInfo(forDate: Date()) { info in
    print(String(describing: info))
}

pic.fetchPhotoInfo(forDates: [Date(), Date().addingTimeInterval(-24*60*60)]) { infos in
    print(infos)
}

And the output is:

Optional(NASA.PhotoInfo(copyright: "Stephane Guisard"))
[NASA.PhotoInfo(copyright: "Stephane Guisard"), NASA.PhotoInfo(copyright: "Zixuan LinBeijing Normal U.")]

There's no error handling, you have to add it yourself.

Even if I did provide an answer to your question, I'm going to mark it as a duplicate of Wait until swift for loop with asynchronous network requests finishes executing. Your code to fetch single photo info works and you are just struggling to understand how to wait for multiple asynchronous tasks.

zrzka
  • 20,249
  • 5
  • 47
  • 73
  • I like how concise the solution is, but at the moment I am not good enough to understand what the code do in detail still I will definitely going back on the future to see if I manage to do it with your example. – Jack1886 Jul 23 '20 at 23:44