Problem with reusing UITableViewCell's

Question

I have a UITableView that is re-using cells when the user scrolls. Everything appears and scrolls fine, except when the user clicks on an actual row, the highlighted cell displays some text from another cell. I'm not exactly sure why.

#define IMAGE_TAG 1111
#define LOGIN_TAG 2222
#define FULL_NAME_TAG 3333

// Customize the appearance of table view cells.
- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath {

    static NSString *CellIdentifier = @"Cell";

    STUser *mySTUser = [[[STUser alloc]init]autorelease];
    mySTUser = [items objectAtIndex:indexPath.row];

    AsyncImageView* asyncImage = nil;
    UILabel* loginLabel = nil;
    UILabel* fullNameLabel = nil;

    UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:CellIdentifier];
    if (cell == nil) {
        cell = [[[UITableViewCell alloc] initWithStyle:UITableViewCellStyleSubtitle reuseIdentifier:CellIdentifier] autorelease];
    }
    else {
        asyncImage = (AsyncImageView *) [cell.contentView viewWithTag:IMAGE_TAG];
        loginLabel = (UILabel *) [cell.contentView viewWithTag:LOGIN_TAG];
        fullNameLabel = (UILabel *) [cell.contentView viewWithTag:FULL_NAME_TAG];
    }

    // Configure the cell...

    cell.accessoryType = UITableViewCellAccessoryDisclosureIndicator;

    CGRect frame = CGRectMake(0, 0, 44, 44);
    asyncImage = [[[AsyncImageView alloc]initWithFrame:frame] autorelease];
    asyncImage.tag = IMAGE_TAG;
    NSURL* url = [NSURL URLWithString:mySTUser.avatar_url_large];
    [asyncImage loadImageFromURL:url];
    [cell.contentView addSubview:asyncImage];

    loginLabel.tag = LOGIN_TAG;
    CGRect loginLabelFrame = CGRectMake(60, 0, 200, 10);
    loginLabel = [[[UILabel alloc] initWithFrame:loginLabelFrame] autorelease];
    loginLabel.text = [NSString stringWithFormat:@"%@",mySTUser.login];
    [cell.contentView addSubview:loginLabel];

    fullNameLabel.tag = FULL_NAME_TAG;
    CGRect fullNameLabelFrame = CGRectMake(60, 20, 200, 10);
    fullNameLabel = [[[UILabel alloc] initWithFrame:fullNameLabelFrame] autorelease];
    fullNameLabel.text = [NSString stringWithFormat:@"%@ %@",mySTUser.first_name, mySTUser.last_name]; //[NSString stringWithFormat:@"%@",mySTUser.login];
    [cell.contentView addSubview:fullNameLabel];    


    return cell;
}

Answer 1

If cell is not nil, you first assign image and labels vars from the cell, and then init them all over again. Right?

Answer 2

This line allocates an object then discards it. Do not allocate an item you will not use.

STUser *mySTUser = [[[STUser alloc]init]autorelease];
mySTUser = [items objectAtIndex:indexPath.row];

Instead, just declare a variable and use it.

STUser *mySTUser;
mySTUser = [items objectAtIndex:indexPath.row];

This line creates a new object and assigns it to the cell, but it happens every time the cell is used.

asyncImage = [[[AsyncImageView alloc]initWithFrame:frame] autorelease];
[cell.contentView addSubview:asyncImage];

Instead, put all the addSubview lines in the if condition where the cell is created.

if (cell == nil) {
    CGRect frame = CGRectMake(0, 0, 44, 44);
    CGRect loginLabelFrame = CGRectMake(60, 0, 200, 10);
    CGRect fullNameLabelFrame = CGRectMake(60, 20, 200, 10);
    cell = [[[UITableViewCell alloc] initWithStyle:UITableViewCellStyleSubtitle reuseIdentifier:CellIdentifier] autorelease];
    asyncImage = [[[AsyncImageView alloc]initWithFrame:frame] autorelease];
    [cell.contentView addSubview:asyncImage];
    loginLabel = [[[UILabel alloc] initWithFrame:loginLabelFrame] autorelease];
    [cell.contentView addSubview:loginLabel];
    fullNameLabel = [[[UILabel alloc] initWithFrame:fullNameLabelFrame] autorelease];
    [cell.contentView addSubview:fullNameLabel];    
    asyncImage.tag = IMAGE_TAG;
    loginLabel.tag = LOGIN_TAG;
    fullNameLabel.tag = FULL_NAME_TAG;
} else ...

The only thing that should happen outside that if block is assignments to properties that change per cell, like [asyncImage loadImageFromURL:url]; and assigning text to the labels.

This line assigns a property to a possibly nil object, then allocates the object.

loginLabel.tag = LOGIN_TAG;
loginLabel = [[[UILabel alloc] initWithFrame:loginLabelFrame] autorelease];

Instead, assign the property after you create the object.

loginLabel = [[[UILabel alloc] initWithFrame:loginLabelFrame] autorelease];
loginLabel.tag = LOGIN_TAG;

This line uses a formatted string where a simple assignment would do.

loginLabel.text = [NSString stringWithFormat:@"%@",mySTUser.login];

Instead, assuming mySTUser.login, just assign it directly.

loginLabel.text = mySTUser.login;