UITableView dequeued cells reloading old images

Question

I know this has been asked many many times but please bear with me as I've tried for a could days to find a solution.

I have an rss parser, a tableview with: text, detail, and images set to the AccessoryDisclosureIndicator.view.

The images load great with simple GCD async: fast scrolling for hundreds of result. NO lag, NO errors if I have a good connection.

The problem is, for a split second - they flicker on load, because the cell is being reused. Also if the connection is poor it will sometimes leave the straggling image BUT the text/detail are correct, only the image is old... So let me repeat, the text/detail updates fine and NEVER gets re-queued wrong, just the image sometimes rarely gets queued wrong on bad connections/seizure-scrolling back and forth fast.

My question, can someone help me cache/tag the cell.accs.views? I tried setting cellIDs but had trouble with my implementation. My below code works great if the connection never slows, just a slight flicker on re-queuing a cell which i don't mind.

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

if (cell == nil) {
    cell = [[UITableViewCell alloc] initWithStyle:UITableViewCellStyleSubtitle reuseIdentifier:CellIdentifier];

    [cell.textLabel setNumberOfLines:3];
    cell.textLabel.font = [UIFont boldSystemFontOfSize:14.0];

    [cell.detailTextLabel setNumberOfLines:3];
    cell.detailTextLabel.font = [UIFont systemFontOfSize:12.0];
    cell.detailTextLabel.textColor = [UIColor blackColor];
}

RSSItem * rssItem = (RSSItem *)(_rssParser.rssItems)[indexPath.row];

dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH,  0ul);
dispatch_async(queue, ^{
    //This is what you will load lazily
    NSData   *data = [NSData dataWithContentsOfURL:[NSURL URLWithString:rssItem.imageURL]];
    dispatch_sync(dispatch_get_main_queue(), ^{

        UIImageView *accImageView = [[UIImageView alloc] initWithImage:[UIImage imageWithData:data ]];

        [accImageView setFrame:CGRectMake(0, 0, accImageView.image.size.width, 92)];
        cell.accessoryView = accImageView;

        //[cell setNeedsLayout];//not needed
    });
});

[cell.textLabel setText:rssItem.title];
[cell.detailTextLabel setText:rssItem.summary];

return cell;}

Answer 1

The key issue is that before you dispatch_async to get the new image (which you know may take a few moments, even seconds on a slow connection), reset the old one or load it with a placeholder image.

Two secondary issues:

1. Be aware that it's possible, if using a very slow network, that by the time the image gets back, that you could have scrolled the cell off the screen (e.g. imagine that you flicked the scroll view up and then, while images were still being loaded, you flicked it back down). You really should make sure that the cell is still visible before trying to set the image.

   You can do this by checking (on the main queue) whether the cell is even visible:

   dispatch_async(dispatch_get_main_queue(), ^{
       UITableViewCell *updateCell = [tableView cellForRowAtIndexPath:indexPath];
       if (updateCell != nil) {
           // update image in `updateCell`
       }
   });
   

   Please note, this UITableView method, cellForRowAtIndexPath, should not be confused with the UITableViewDataSource method, tableView:cellForRowAtIndexPath:. The former simply checks to see if the cell is visible, and if so, returns a pointer to it. The latter, obviously, is the method that you reference in your question, for creating and configuring table view cells.

   Also note that this idea, that you use the NSIndexPath to check to see if the cell is still visible, assumes that no additional rows could have possibly been inserted in the table view in the intervening time. This often is not a valid assumption, so you may even have to go back to the underlying model and re-calculate the indexPath for this cell before proceeding with the above logic to see if that cell is still visible or not.

2. You might not want to use a global queue because on really slow network your requests might start to get backlogged and you might end up with a lot of concurrent network requests. This is a problem because (a) the system can't do more than 4 or 5 concurrent requests at a time, anyway; and (b) you'll be using up the limited number of worker threads provided by iOS. You really shouldn't have more than 4 or 5 concurrent requests (if you have more than that, not only will you see no performance gain, but they'll actually block and eventually timeout). I'd suggest you consider using a NSOperationQueue for which you set maxConcurrentOperationCount to four or five.

   Also, this affords one final optimization, notably making these NSOperation subclass requests cancelable (and, obviously, making the cancelation logic actually cancel the network request contained therein). Thus, if you scroll quickly to the 100th row in the table, you really don't want to have the visible cells waiting for the image downloads for the previous 99 rows to finish first. If you cancel the image request for cells that scrolled off screen, that resolves this problem.

If you want to diagnose your app's behavior on a really slow network connection, I'd suggest that you get the Network Conditioner, which can have your Mac simulate anything ranging from full speed to a really poor quality Edge network on the simulator. In Xcode menu in Xcode, choose "Open Developer Tool..." and then pick "More Developer Tools". After you log in, you'll see an option for "Hardware IO Tools for Xcode". The Network Conditioner tool is there. You can test your app in worst case scenarios, which may get you thinking about not only the two problems I outline above, but also more advanced features like caching of images and the like.

You really want to cache images, at the very least to persistent storage, ideally to both RAM and persistent storage. There are UIImageView categories that tackle both the asynchronous issues above, as well as offer some caching: Two that you might consider are SDWebImage and AFNetworking.

Answer 2

You just need to clear out the old image when reusing the cell.

At the end, you can just add:

cell.accessoryView = nil;

Answer 3

The problem is, for a split second - they flicker on load, because the cell is being reused

But that's your answer right there. The cell is being reused, and I don't see anything in your code that removes the image. If the image is not the right image for this row (because it is being reused in a different row), you need to set cell.accessoryView to nil (or to an image view with a placeholder image) now. The right image won't come along until later (dispatch_async).

However, the code you are using is highly flawed, because it is fetching the image from the Internet every time, even if we've already seen this row and therefore already downloaded the image. My book has much better code for loading the images in a table view by downloading them if necessary and storing them thereafter. It also shows you how to stop the download if the row is scrolled off the table before the image arrives, thereby saving bandwidth:

http://www.apeth.com/iOSBook/ch37.html#_http_requests

Scroll down to where it says "Suppose, for example, you need to download images to serve as thumbnails in the cells of a UITableView. Let’s consider how these images can be supplied lazily on demand."