Asynchronous downloading of images for UITableView with GCD

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;
}

Hedges

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;
}

Mack

@SeamusCampbell, couldn't indexPath.row be the same for the reused cell if it's in a different section? In that case the tag check would pass inappropriately. – Masturbation

@SeamusCampbell, any thoughts for a multi-section table :) – Masturbation

@AndrewRobinson You could have the tag be a concatenation of the section and row. It could be "section:row", for example. – Dyspeptic

@eyuelt, just another quick note: The concatenation of section & row only worked for me because I could guarantee the # of rows wouldn't cause duplication. If the rows are not predictable, you could run into the issue where the concatenation is the same for two index paths, think section 1 row 11 and section 11 row 1. – Masturbation

@AndrewRobinson That was why I suggested "section:row". If you use a separator between the section and row, like ":", you won't have that issue. – Dyspeptic

@Dyspeptic I don't believe you can have a ":" character in a view's tag? – Masturbation

hey if I wanted to just have an image in a regular view, where would I call this code in viewDidLoad or viewDidAppear ? – Bombshell

for multi-section table, wouldn't be safer to use indexPath.hash? – Tinny

This idea of using the indexPath to check if it s the same cell is not good. if (cell.tag == indexPath.row) { WHY? Because you can have the same cell reused at the exact same position. Basically you can't assume to know where the cell will be reused. One good solution here is to store the image url in the cell and check if it s the same before setting the UIImageView. – International

Is there any way around this? I'm having an issue where the cell does not match and I'm forced to refresh again and again until it does which isn't practical. Is there a way to assure the cell matches? – Ihs

Answer 2

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.

Spumescent

Answer 3

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;
}

Viaticum

Just a note, this doesn't work with very fast scrolling. It's still possible to see an image added to an incorrect cell. If you call [tableView cellForRowAtIndexPath:indexPath] right before the cell is reused, the call to blockCell.imageView.image = image may very well get called for the cell after it's reused. – Masturbation

@AndrewRobinson if it doesn't work with fast scrolling, then it doesn't work. But, I've not seen this approach fail in the way you describe, myself - could you elaborate on how you determined that the cell was being re-used for a different index path while executing the block? – Viaticum

@AndrewRobinson if could post a sample project on github or something I could play with I'd love to investigate, as well; beyond looking at your project though I don't have any insight off the top of my head, unfortunately. – Viaticum

why do u duplicate useless info? it is not appropriate for table view! – Sheep

Answer 4

I've been studying about this problem and I found an excellent approach by customizing an UITableViewCell.

#import <UIKit/UIKit.h>

@interface MyCustomCell : UITableViewCell

@property (nonatomic, strong) NSURLSessionDataTask *imageDownloadTask;
@property (nonatomic, weak) IBOutlet UIImageView *myImageView;
@property (nonatomic, weak) IBOutlet UIActivityIndicatorView *activityIndicator;

@end

Now, in your TableViewController, declare two properties for NSURLSessionConfiguration and NSURLSession and initialize them in ViewDidLoad:

@interface MyTableViewController ()

@property (nonatomic, strong) NSURLSessionConfiguration *sessionConfig;
@property (nonatomic, strong) NSURLSession *session;
.
.
.
@end

@implementation TimesVC

- (void)viewDidLoad
{
    [super viewDidLoad];

    _sessionConfig = [NSURLSessionConfiguration defaultSessionConfiguration];
    _session = [NSURLSession sessionWithConfiguration:_sessionConfig];
}

.
.
.

Let's supose that your datasource is a NSMutableDictionary's array (or NSManagedObjectContext). You can easilly download image for each cell, with caching, like that:

.
.
.
- (MyCustomCell *)tableView:(UITableView *)tableView
   cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    MyCustomCell *cell = (MyCustomCell *)[tableView dequeueReusableCellWithIdentifier:@"cell" forIndexPath:indexPath];

    if (!cell)
    {
        cell = [[MyCustomCell alloc] initWithStyle:UITableViewCellStyleDefault
                                reuseIdentifier:@"cell"];
    }

    NSMutableDictionary *myDictionary = [_myArrayDataSource objectAtIndex:indexPath.row];    

    if (cell.imageDownloadTask)
    {
        [cell.imageDownloadTask cancel];
    }

    [cell.activityIndicator startAnimating];
    cell.myImageView.image = nil;

    if (![myDictionary valueForKey:@"image"])
    {
        NSString *urlString = [myDictionary valueForKey:@"imageURL"];
        NSURL *imageURL = [NSURL URLWithString:urlString];
        if (imageURL)
        {
            cell.imageDownloadTask = [_session dataTaskWithURL:imageURL
                completionHandler:^(NSData *data, NSURLResponse *response, NSError *error)
            {
                if (error)
                {
                    NSLog(@"ERROR: %@", error);
                }
                else
                {
                    NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)response;

                    if (httpResponse.statusCode == 200)
                    {
                        UIImage *image = [UIImage imageWithData:data];

                        dispatch_async(dispatch_get_main_queue(), ^{
                            [myDictionary setValue:data forKey:@"image"];
                            [cell.myImageView setImage:image];
                            [cell.activityIndicator stopAnimating];
                        });
                    }
                    else
                    {
                        NSLog(@"Couldn't load image at URL: %@", imageURL);
                        NSLog(@"HTTP %d", httpResponse.statusCode);
                    }
                }
            }];

            [cell.imageDownloadTask resume];
        }
    }
    else
    {
        [cell.myImageView setImage:[UIImage imageWithData:[myDictionary valueForKey:@"image"]]];
        [cell.activityIndicator stopAnimating];
    }

    return cell;
}

I hope it helps some devs! Cheers.

CREDITS: Table View Images in iOS 7

Vair

Answer 5

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

Humo

Answer 6

Don't reinvent the wheel, just use this couple of awesome pods:

https://github.com/rs/SDWebImage

https://github.com/JJSaccolo/UIActivityIndicator-for-SDWebImage

As simple as:

    [self.eventImage setImageWithURL:[NSURL URLWithString:[NSString stringWithFormat:@"%@/%@", [SchemeConfiguration APIEndpoint] , _event.imageUrl]]
                          placeholderImage:nil
                                 completed:^(UIImage *image,
                                             NSError *error,
                                             SDImageCacheType cacheType,
                                             NSURL *imageURL)
     {
         event.image = UIImagePNGRepresentation(image);
     } usingActivityIndicatorStyle:UIActivityIndicatorViewStyleWhite];

Strickman

It was interview constraint to implement this without third party libraries. – Hedges

I'd add that, when possible, you should avoid using CocoaPods to solve issues for you without understanding them - that's one of the hallmarks of a good engineer. That being said, data/image caching is a very hard problem that should probably be solved with a 3rd-party library. If the one you choose provides this functionality, by all means, use it! – Brough

Yes, I am agree with you @dinesharjani. A good engineer should be aware of what the third party library is doing for him and how it is doing it. In that case the library worked perfect for me regarding response times, etc. That's why I decided to use it. – Strickman

Answer 7

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)
              }
          });
     }

Gur

Answer 8

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)
    }

Natalianatalie