iPhone: Cell imageViews repeating when loaded from url

Question

I'm developing a demo RSS reader for iPhone. I obviously have a tableview to display the feeds, and then a detailed view. Some of this feeds have a thumbnail that I want to display on cell.imageview of the table, and some don't.

The problem is that when scrolling the table, loaded thumbnails start repeating on other cells, and I end up with a thumbnail on every cell.

Here's a piece of my code. I may upload screenshots later

   - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath{
        UITableViewCell * cell = [tableView dequeueReusableCellWithIdentifier: @"rssItemCell"];
        if(nil == cell){
            cell = [[[UITableViewCell alloc] initWithStyle:UITableViewCellStyleSubtitle reuseIdentifier:@"rssItemCell"]autorelease];
        }
        BlogRss *item = [[[self rssParser]rssItems]objectAtIndex:indexPath.row];
        cell.textLabel.text = [item title];
        cell.detailTextLabel.text = [item description];
        cell.accessoryType = UITableViewCellAccessoryDisclosureIndicator;

        // Thumbnail if exists
        if(noticia.imagePath != nil){
            NSData* imageData;
            @try {
                imageData = [[NSData alloc]initWithContentsOfURL:[NSURL URLWithString:item.imagePath]];
            }
            @catch (NSException * e) {
                //Some error while downloading data
            }
            @finally {
                item.image = [[UIImage alloc] initWithData:imageData];
                [imageData release];
            }
        }

        if(item.image != nil){
            [[cell imageView] setImage:item.image];
        }
        return cell;
    }

Any help will be very appreciated.

Answer 1

Easy one.

Cells are reused. Just ensure you clear the cell.imageView.image each time you fill the cell. You may just need to remove the if(item.image!=nil) line.

For a production application you probably also want to fetch the images in the background and implement a simple cache. There are plenty of examples of how to do that knocking around.

EDIT

RickiG makes a lot of good points : cellForRowAtIndexPath should be displaying the data from the model and as little as possible else!

The concept of just supplying a view-ready model is kind of good (I do it with ASP.NET MVC), but needs to be balanced against the JustInTime memory minimization techniques of iPhone and lets face it, you are not committing the real sin of trying to read back data from the controls on your tableview - that really doesn't work!

The worst thing you are doing is reading web data on cellForRowAtIndexPath as that will block the UK. Instead you should display a blank or placeholder image and trigger a background fetch that will update the model with the data, and then trigger a reload - preferably of the specific cell.

Answer 2

The UITableView is made to reflect a "model", so never try to change data/views on the cell is self after you build it or reference the cell based on an index number or the like, make an array of your data, I usually build a separate CellViewEntity object that holds all the data I need on the cell, like title, detailtext etc. but also behavior stuff like, is it expanded, has it got special view visible etc.

I then build a UIView CellView that I populate with the graphics and methods that I need, e.g. - (void) shouldDisplayCheckmark:(BOOL) value and so on. I set the tag of the CellVIew object - [cellView setTag:15] and release the CellView. Now I can reference it later without retaining it and let the UITableVIew decide what should be released/retained.

cellForRowAtIndexPath is called constantly by the SDK, when scrolling when updating, when a cell enter or leave the screen. So don't put heavy instantiations, web call etc. into this function.

In this if block.

   if(nil == cell){
      cell = [[[UITableViewCell alloc] initWithStyle:UITableViewCellStyleSubtitle reuseIdentifier:@"rssItemCell"]autorelease];
        }

I instantiate a cell and a CellView and add the CellView object to the cells view. (with the tag)

outside the if(cell == nil) I only update the CellVIew with the data from the model array.

This means that if the cell exists it will be reused and have its properties updated from the array, if not it is created and attached to the cell. Outside the if statement I reference my CellView like this:

[(CellView*)[cell viewWithTag:15] updateValuesAccordingToModelArray:[array objectAtIndes:indexPath.row]]; //this will be the only code executed for a already existing cell.

I guess it might seem a bit over the limit, but you can not reference cells like this [uitableView cellAtRow:14] in a consistent way, because the UITableView caches cell that are off screen and changes the indexPath accordingly. This approach you can do with your array.

Will stop rambling now :) ... separate all data in a nice tableView friendly package and feed it to the table view - one way only.