asynchronously load images from URL

Question

I am loading images into a UICollectionView with Swift. I am getting the images asynchronously. While the UI is scrolling and not locking up, there are still some problems. I am getting some repeats, and they are loading into the view fairly slow. I created a video to show exactly the problem:

https://www.youtube.com/watch?v=nQmAXpbrv1w&feature=youtu.be

My code takes mostly from the accepted answer to this question: Loading/Downloading image from URL on Swift.

Here is my exact code.

Below my UICollectionViewController class declaration, I declare a variable:

let API = APIGet()

In the viewDidLoad of the UICollectionViewController, I call:

 override func viewDidLoad() {
    API.getRequest()
    API.delegate = self
}

In my APIGet class, I have the function, getRequest:

  func getRequest() {
let string = "https://api.imgur.com/3/gallery/t/cats"
let url = NSURL(string: string)
let request = NSMutableURLRequest(URL: url!)
request.setValue("Client-ID \(cliendID)", forHTTPHeaderField: "Authorization")
let session = NSURLSession.sharedSession()
let tache = session.dataTaskWithRequest(request) { (data, response, error) -> Void in
    if let antwort = response as? NSHTTPURLResponse {
        let code = antwort.statusCode
        print(code)
        do {

            let json = try NSJSONSerialization.JSONObjectWithData(data!, options: NSJSONReadingOptions.MutableContainers)

            dispatch_async(dispatch_get_main_queue(), { () -> Void in
                if let data = json["data"] as? NSDictionary {
                    if let items = data["items"] as? NSArray {
                        var x = 0
                        while x < items.count {
                            if let url2 = items[x]["link"] {
                                self.urls.append(url2! as! String)


                            }

                            x++
                        }
                        self.delegate?.retrievedUrls(self.urls)

                    }
                }

            })



        }
        catch {

        }
    }
   }
 tache.resume()

 }

The above function does exactly what it should: It gets the JSON from the endpoint, and gets the URLs of the images from the JSON. Once all the URLs are retrieved, it passes them back to the UICollectionViewController.

Now, for my code in the UICOllectionViewController. This is the function that receives the URLs from the APIGet class.:

  func retrievedUrls(x: [String]) {
    //
    self.URLs = x
    var y = 0
    while y < self.URLs.count {
        if let checkedURL = NSURL(string: self.URLs[y]) {
            z = y
            downloadImage(checkedURL)

        }
        y++
    }
}

Then, these 2 functions take care of the downloading of the image:

   func getDataFromUrl(url:NSURL, completion: ((data: NSData?, response: NSURLResponse?, error: NSError? ) -> Void)) {
    NSURLSession.sharedSession().dataTaskWithURL(url) { (data, response, error) in
        completion(data: data, response: response, error: error)
        }.resume()

}


func downloadImage(url: NSURL){
    getDataFromUrl(url) { (data, response, error)  in
        dispatch_async(dispatch_get_main_queue()) { () -> Void in
            guard let data = data where error == nil else { return }

            if let image = UIImage(data: data) {
                self.images.append(image)
                self.collectionView?.reloadData()
            }
        }
    }
}

When I call self.collectionView.reloadData() after the image is loaded, then every time a new image is loaded, the collectionView flashes and the cells shift around. Yet, if I don't call self.collectionView.reloadData() at all, then the collectionView never reloads after the images are loaded, and the cells remain empty. How can I get around this? I want the images to just load right into their respective cells, and not have the collectionView blink/flash or have cells shift around, or get temporary duplicates.

My cellForRowAtIndexPath is simply:

  override func collectionView(collectionView: UICollectionView, cellForItemAtIndexPath indexPath: NSIndexPath) -> UICollectionViewCell {
    let cell = collectionView.dequeueReusableCellWithReuseIdentifier("cell1", forIndexPath: indexPath) as! cell1

    if indexPath.row > (images.count - 1) {
        cell.image.backgroundColor = UIColor.grayColor()
    }
    else {
    cell.image.image = images[indexPath.row]
    }


    return cell
}

This is my first foray into networking in iOS, so I would prefer not to default to a library (alamofire, AFNetowrking, SDWebImage, etc), so I can learn how to do it from the ground up. Thanks for reading all this!

EDIT: Here is my final code, taken mostly from the answer below:

    func retrievedUrls(x: [String]) {
    //
    self.URLs = x
    self.collectionView?.reloadData()
    var y = 0
    while y < self.URLs.count {
        if let checkedURL = NSURL(string: self.URLs[y]) {

            downloadImage(checkedURL, completion: { (image) -> Void in

            })

        }
        y++
    }
}

func getDataFromUrl(url:NSURL, completion: ((data: NSData?, response: NSURLResponse?, error: NSError? ) -> Void)) {
    NSURLSession.sharedSession().dataTaskWithURL(url) { (data, response, error) in
        completion(data: data, response: response, error: error)
        }.resume()

}

func downloadImage(url: NSURL, completion: (UIImage) -> Void) {
    getDataFromUrl(url) { (data, response, error) -> Void in
        dispatch_async(dispatch_get_main_queue()) { () -> Void in
            guard let data = data where error == nil else { return }

            if let image = UIImage(data: data) {
                self.images.append(image)
                let index = self.images.indexOf(image)
                let indexPath = NSIndexPath(forRow: index!, inSection: 0)
                self.collectionView?.reloadItemsAtIndexPaths([indexPath])


            }
        }
    }
}

My cellForItemAtIndexPath didn't change.

Answer 1

I'm not sure how familiar you are with closures and the like, but you are using them as part of NSURLSession so I'll assume a passing familiarity.

In your function downloadImage(_:) you could rewrite it instead to have the signature

func downloadImage(url: NSURL, completion: (UIImage) -> Void)

Which allows you to at-time-of-calling decide what else to do with that image. downloadImage would be modified as:

func downloadImage(url: NSURL, completion: (UIImage) -> Void) {
  getDataFromUrl(url) { (data, response, error) -> Void in
    dispatch_async(dispatch_get_main_queue()) { () -> Void in
      guard let data = data where error == nil else { return }

      if let image = UIImage(data: data) {
        self.images.append(image)
        completion(image)
      }
    }
  }
}

All that we changed, is we pass our image into the newly defined completion block.

Now, why is this useful? retreivedUrls(_:) can be modified in one of two ways: either to assign the image to the cell directly, or to reload the cell at the corresponding index (whichever is easier; assigning directly is guaranteed not to cause flicker, though in my experience, reloading a single cell doesn't either).

func retrievedUrls(x: [String]) {
  //
  self.URLs = x
  var y = 0
  while y < self.URLs.count {
    if let checkedURL = NSURL(string: self.URLs[y]) {
      z = y
      downloadImage(checkedURL, completion: { (image) -> Void in
        // either assign directly:
        let indexPath = NSIndexPath(forRow: y, inSection: 0)
        let cell = collectionView.cellForItemAtIndexPath(indexPath) as? YourCellType
        cell?.image.image = image
        // or reload the index
        let indexPath = NSIndexPath(forRow: y, inSection: 0)
        collectionView?.reloadItemsAtIndexPaths([indexPath])
      })

    }
    y++
  }
}

Note that we use the completion block, which will only be called after the image has been downloaded and appended to the images array (same time that collectionView?.reloadData() used to be called).

However! Of note in your implementation, you are appending images to the array, but may not be maintaining the same ordering as your URLs! If some images are not downloaded in the exactly same order as that in which you pass in URLs (which is quite likely, since it's all async), they will get mixed up. I don't know what your application does, but presumably you will want images to be assigned to cells in the same order as the URLs you were given. My suggestion is to change your images array to be of type Array<UIImage?>, and filling it with the same amount of nils as you have URLs, and then modifying cellForItemAtIndexPath to check if the image is nil: if nil, assign the UIColor.grayColor(), but if it's non-nil, assign it to the cell's image. This way you maintain ordering. It's not a perfect solution, but it requires the least modification on your end.