iphone - cellForRowAtIndexPath memory management

Question

So, here's the code for my cellForRowAtIndexPath:

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

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

    NSInteger artistIndex = 1; // 1
    NSInteger albumIndex = 3;  // 3
    NSInteger dateIndex = 6;   // 6
    NSInteger imageIndex = 8;  // 5

    // ARTIST
    CGRect frame = CGRectMake(59, 11, 244, 13);
    UILabel *label = [[UILabel alloc]initWithFrame:frame];
    label.font = [UIFont boldSystemFontOfSize:13];
    label.textColor = [UIColor blackColor];
    label.text = [[musicList.list objectAtIndex:indexPath.row] objectAtIndex:artistIndex];
    [cell addSubview:label];

    // ALBUM (more like description...
    frame = CGRectMake(60, 30, 244, 11);
    label = [[UILabel alloc]initWithFrame:frame];
    label.font = [UIFont boldSystemFontOfSize:11];
    label.textColor = [UIColor darkGrayColor];
    label.text = [[musicList.list objectAtIndex:indexPath.row] objectAtIndex:albumIndex];
    [cell addSubview:label];

    // DATE
    frame = CGRectMake(59, 49, 244, 10);
    label = [[UILabel alloc]initWithFrame:frame];
    label.font = [UIFont fontWithName:@"Helvetica" size:10.0];
    label.textColor = [UIColor darkGrayColor];
    label.textAlignment = UITextAlignmentRight;
    label.text = [[musicList.list objectAtIndex:indexPath.row] objectAtIndex:dateIndex];
    [cell addSubview:label];

    // IMAGE
    UIImageView *imageView = [[UIImageView alloc]init];
    imageView.image = [[musicList.list objectAtIndex:indexPath.row] objectAtIndex:imageIndex];
    imageView.frame = CGRectMake(8,9,44,44);
    [imageView.layer setMasksToBounds:YES];
    [imageView.layer setCornerRadius:3.0];
    [[cell contentView] addSubview:imageView];

    [imageView release];
    [label release]; 

    return cell;
}

To explain what's going on, basically it just taps into an array that stores strings for everything except for index 8 (which stores a UIImage).

I have no variables set to autorelease (mostly because I don't really understand autorelease just yet lol)

Anyways, when scrolling through the app, it will slow down progressively (mostly because of the UIImage, but also in part because of the labels and frames).

Is there anyway to manage my memory better? Also, is there a nicer way of coding this? I was thinking of making an array of UITableViewCells and just accessing those via cellForRowAtIndexPath.

So I would love for some suggestions, thanks guys.

Answer 1

You're leaking a lot of labels there. Every time you alloc/init a new label you need to release it. Currently you are re-using the same label variable several times by assigning it a new label object without releasing the old one.

Instead of putting your [label release] at the end, put [label release] after every [cell addSubview:label];

Your next problem is you've misunderstood how the table cell recycling works. UITableCell objects are reused over and over again within a table, that why you do this bit:

UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:@"identifier"];

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

That's basically saying "check if there's a cell available, and if not create a new one". When a cell is re-used it keeps all the old stuff you've added to it. So when you add those labels and image view to the cell, they stay in that cell the next time it is used, and the next time.

But because you add the labels again each time the cell is re-used, it builds up multiple copies of the labels, so the second time the cell is displayed it has twice as many labels, and the next time 3 times as many, and so on. You can't see the duplicates because they're in exactly the same place, but they're there using up memory and slowing your table down.

You need to move the code where you append new views to the cell inside the "if (cell == nil) { ... }" statement, so the labels are only added once when the cell is created.

Of course that means that every cell will have the same text and image, which you don't want, so you need to split out the logic where you set the text and images and put it after the if statement. That way you only create the labels once but you set the text each time they are displayed in a different table row. That's a bit tricky because you don't have the references to the labels any more, but you can do it by setting unique tags on the labels so you can find them again.

Here's a cleaned-up version with all the leaks removed:

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath {

    UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:@"identifier"];

    NSInteger artistTag = 1;
    NSInteger albumTag = 2;
    NSInteger dateTag = 3;
    NSInteger imageTag = 4;

    if (cell == nil) {

        cell = [[[UITableViewCell alloc] initWithStyle:UITableViewCellStyleSubtitle reuseIdentifier:@"identifier"]autorelease];

        // ARTIST
        CGRect frame = CGRectMake(59, 11, 244, 13);
        UILabel *label = [[UILabel alloc]initWithFrame:frame];
        label.font = [UIFont boldSystemFontOfSize:13];
        label.textColor = [UIColor blackColor];
        label.tag = artistTag;
        [cell addSubview:label];
        [label release];

        // ALBUM (more like description...
        frame = CGRectMake(60, 30, 244, 11);
        label = [[UILabel alloc]initWithFrame:frame];
        label.font = [UIFont boldSystemFontOfSize:11];
        label.textColor = [UIColor darkGrayColor];
        label.tag = albumTag;
        [cell addSubview:label];
        [label release];

        // DATE
        frame = CGRectMake(59, 49, 244, 10);
        label = [[UILabel alloc]initWithFrame:frame];
        label.font = [UIFont fontWithName:@"Helvetica" size:10.0];
        label.textColor = [UIColor darkGrayColor];
        label.textAlignment = UITextAlignmentRight;
        label.tag = dateTag;
        [cell addSubview:label];
        [label release];

        // IMAGE
        UIImageView *imageView = [[UIImageView alloc]init];
        imageView.frame = CGRectMake(8,9,44,44);
        imageView.tag = imageTag;
        [imageView.layer setMasksToBounds:YES];
        [imageView.layer setCornerRadius:3.0];
        [[cell contentView] addSubview:imageView];
        [imageView release];
    }

    NSInteger artistIndex = 1; // 1
    NSInteger albumIndex = 3;  // 3
    NSInteger dateIndex = 6;   // 6
    NSInteger imageIndex = 8;  // 5

    ((UILabel *)[cell viewWithTag:artistTag]).text = [[musicList.list objectAtIndex:indexPath.row] objectAtIndex:artistIndex];

    ((UILabel *)[cell viewWithTag:albumTag]).text = [[musicList.list objectAtIndex:indexPath.row] objectAtIndex:albumIndex];

    ((UILabel *)[cell viewWithTag:dateTag]).text = [[musicList.list objectAtIndex:indexPath.row] objectAtIndex:dateIndex];

    ((UIImageView *)[cell viewWithTag:imageTag]).image = [[musicList.list objectAtIndex:indexPath.row] objectAtIndex:imageIndex];


    return cell;
}