Search code examples
iosobjective-cuitableviewnsurl

Loading images asynchronously, weird problems


There is my question:

i have custom class that parse through an XML and get string i need to use as URL for my strings, now i modified my code as follow:

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

UILabel *labelText = (UILabel *)[cell viewWithTag:1000];
    labelText.text = [[self.listOfPlaceDetails objectAtIndex:indexPath.row] objectForKey:@"name"];




    dispatch_queue_t concurrentQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
    dispatch_async(concurrentQueue, ^{
        NSURL *imageURL = [NSURL URLWithString:[[self.listOfPlaceDetails objectAtIndex:indexPath.row]objectForKey:@"imageCell"]];

        NSData *image = [[NSData alloc] initWithContentsOfURL:imageURL];

        dispatch_async(dispatch_get_main_queue(), ^{



            cell.imageView.image = [UIImage imageWithData:image];
        });
    });

 return cell;
}

This is pretty straightforward, but, i got unpredictable errors! During scrolling table, images start to chaotically change, sometimes it show 3 or more images and final image is correct one, sometimes final (correct) image does not appear at all. Also, when table is first shown, its actually blank, so i need to scroll it bottom, and then up again to see my images!

In attempt to fix that, i add following code, to determine is my image link correct for that indexPath:

-(void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath{

    NSURL *imageURL = [NSURL URLWithString:[[self.listOfPlaceDetails objectAtIndex:indexPath.row]objectForKey:@"imageCell"]];
    NSLog(@"%@", imageURL);
}

And when i tap to any cell, it does show me proper link in console log, but image on cell is one of the image shown before (invalid), and it is not the image for that link. How to fix that weird errors?

Any advice would be appreciated, thank you.


Solution

  • When you dequeue a cell object, most of the time you'll get a reused cell i.e. a cell that has been configured by tableView:cellForRowAtIndexPath: once or more times before.

    To visualise what's happening in your case, consider one likely sequence of events for a single cell as you perform a long, quick scroll:

    1. The cell is created and an image load is spun off in the background
    2. The cell is scrolled off screen, so added to the table view's cache, ready for dequeuing. The image loading is not canceled at this point
    3. The cell is dequeued and an image load is spun off in the background
    4. Steps 2 and 3 are repeated a few times
    5. The cell is visible, but the several image loading tasks are now completing and each is updating the cell's imageView with the loaded image. This will indeed look like the images are chaotically changing as each loading operation finishes.

    (What's more, with a concurrent queue, there's no guarantee that the image loads will complete in the order that they're started - you may not end up with the correct final image!)

    So what do we do about it? Now that we understand the problem, there are lots of different solutions. A very simple solution (that I don't really recommend) is to check that the cell's label text matches the value for that indexPath, when you come to set the image:

    if ([labelText.text isEqualToString:[[self.listOfPlaceDetails objectAtIndex:indexPath.row] objectForKey:@"name"]]) {
       cell.imageView.image = [UIImage imageWithData:image];
    }
    

    Obviously, this assumes that all the place details have unique names.

    A better solution might be to create an object that handles the image download, and is something that you can register/unregister cells against to handle download completion. This object could enforce the condition that a cell cannot be waiting for more than one image load. As @Leena pointed out, caching is a good idea and this object could be responsible for that too.

    As for the blank images, calling [cell setNeedsLayout] after setting the image should sort that out.