Asynchronous downloading of images for UITableView

Question

I'm downloading images for my uitableview asynchronously using GCD, but there is a problem - when scrolling images flicker and change all the time. I tried setting image to nil with every cell, but it doesn't help much. When scrolling back up fast, all images are wrong. What can I do about that? Here is my method for cells:

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    static NSString *CellIdentifier = @"Cell";
    UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:CellIdentifier forIndexPath:indexPath];

    if (self.loader.parsedData[indexPath.row] != nil)
    {
        cell.imageView.image = nil;
        dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0ul);
            dispatch_async(queue, ^(void) {

                NSData *imageData = [NSData dataWithContentsOfURL:[NSURL URLWithString:[self.loader.parsedData[indexPath.row] objectForKey:@"imageLR"]]];

                UIImage* image = [[UIImage alloc] initWithData:imageData];

                dispatch_async(dispatch_get_main_queue(), ^{
                    cell.imageView.image = image;
                    [cell setNeedsLayout];
                     });
            });

    cell.textLabel.text = [self.loader.parsedData[indexPath.row] objectForKey:@"id"];
    }
    return cell;
}

Answer 1

The problem here is that your image-fetching blocks are holding references to the tableview cells. When the download completes, it sets the imageView.image property, even if you have recycled the cell to display a different row.

You'll need your download completion block to test whether the image is still relevant to the cell before setting the image.

It's also worth noting that you're not storing the images anywhere other than in the cell, so you'll be downloading them again each time you scroll a row onscreen. You probably want to cache them somewhere and look for locally cached images before starting a download.

Edit: here's a simple way to test, using the cell's tag property:

- (UITableViewCell *)tableView:(UITableView *)tableView 
         cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    static NSString *CellIdentifier = @"Cell";
    UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:CellIdentifier forIndexPath:indexPath];

    cell.tag = indexPath.row;
    NSDictionary *parsedData = self.loader.parsedData[indexPath.row];
    if (parsedData)
    {
        cell.imageView.image = nil;
        dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0ul);
        dispatch_async(queue, ^(void) {

            NSData *imageData = [NSData dataWithContentsOfURL:[NSURL URLWithString:parsedData[@"imageLR"]];

            UIImage* image = [[UIImage alloc] initWithData:imageData];
            if (image) {
                 dispatch_async(dispatch_get_main_queue(), ^{
                     if (cell.tag == indexPath.row) {
                         cell.imageView.image = image;
                         [cell setNeedsLayout];
                     }
                 });
             }
        });

        cell.textLabel.text = parsedData[@"id"];
    }
    return cell;
}

Answer 2

Have you thought of using SDWebImage? It downloads the image from the given URL asynchronously also. I didn't use the entire library (imported only UIImageView+WebCache.h). Once imported, all you have to do is call the method as such:

[UIImageXYZ sd_setImageWithURL:["url you're retrieving from"] placeholderImage:[UIImage imageNamed:@"defaultProfile.jpeg"]];

It might be overkill if you're using AFNetworking 2.0, but it worked out for me.

Here is the link to the github if you want to give it a try

Answer 3

Beside accepted answer of Seamus Campbell you should also know that sometime this does not work. In that case we should reload the specific cell. So

if (image) {
     dispatch_async(dispatch_get_main_queue(), ^{
          if (cell.tag == indexPath.row) {
               cell.imageView.image = image;
               [cell setNeedsLayout];
          }
      });
 }

should be changed into,

    if (image) {
         dispatch_async(dispatch_get_main_queue(), ^{
              if (cell.tag == indexPath.row) {
                   cell.imageView.image = image;
                   self.tableView.reloadRowsAtIndexPaths([indexPath], withRowAnimation: UITableViewRowAnimation.None)
              }
          });
     }

Answer 4

For those worrying about index paths changing and not being consistent, a potential solution is to subclass UITableViewCell or UICollectionViewCell and add an instance variable called something like stringTag. Then, put the URL of the photo you are downloading in the stringTag. When setting the image, check if the stringTag is still the right URL.

See this answer for more detail: Asynchronous Fetch completed: Is cell still being displayed?.

Here are my classes:

import Foundation
import UIKit

class ImageAsyncCollectionViewCell : UICollectionViewCell {
  var stringTag: String?
}

and then when using the cell:

    cell.stringTag = photoKey
    cell.imageView.image = self.blankImage
    if ImageCache.default.imageCachedType(forKey: photoKey).cached {
      ImageCache.default.retrieveImage(forKey: photoKey, options: nil) {
        image, cacheType in
        if let image = image {
          DispatchQueue.main.async {
            if cell.stringTag == photoKey {
              cell.imageView.image = image
            }
          }
        } else {
          print("Doesn't exist in cache, but should")
          self.setCellWithPhoto(photoKey: photoKey, cell: cell)
        }
      }
    } else {
      self.setCellWithPhoto(photoKey: photoKey, cell: cell)
    }

Answer 5

The point is that you did not fully understand the cell reusing concept. That does not agree very well with asynchronous downloads.

The block

    ^{
    cell.imageView.image = image;
    [cell setNeedsLayout];
}

gets executed when the request is finished and all data was loaded. But cell gets its value when the block is created.

By the time when the block is executed cell still points to one of the existing cells. But it is quite likely that the user continued scrolling. The cell object was re-used in the meantime and the image is associated with an 'old ' cell that is reused and assigned and displayed. Shortly after that the correct image is loaded and assigned and displayed unless the user has scrolled further. and so on and so on.

You should look for a smarter way of doing that. There are lots of turorials. Google for lazy image loading.

Answer 6

Use the index path to get the cell. If it's not visible the cell will be nil and you won't have an issue. Of course you'll probably want to cache the data when it's downloaded so as to set the cell's image immediately when you have the image already.

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    static NSString *CellIdentifier = @"Cell";
    UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:CellIdentifier forIndexPath:indexPath];

    if (self.loader.parsedData[indexPath.row] != nil)
    {
        cell.imageView.image = nil;
        dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0ul);
            dispatch_async(queue, ^(void) {
                //  You may want to cache this explicitly instead of reloading every time.
                NSData *imageData = [NSData dataWithContentsOfURL:[NSURL URLWithString:[self.loader.parsedData[indexPath.row] objectForKey:@"imageLR"]]];
                UIImage* image = [[UIImage alloc] initWithData:imageData];
                dispatch_async(dispatch_get_main_queue(), ^{
                    // Capture the indexPath variable, not the cell variable, and use that
                    UITableViewCell *blockCell = [tableView cellForRowAtIndexPath:indexPath];
                    blockCell.imageView.image = image;
                    [blockCell setNeedsLayout];
                });
            });
        cell.textLabel.text = [self.loader.parsedData[indexPath.row] objectForKey:@"id"];
    }

    return cell;
}