UITableView weird behavior when scrolling

Question

I have created a UITableView which contains cells that display Users. Each cell is added within this method -tableView:cellForRowAtIndexPath:. And each cell has content linked to the specific user, like an UIImageView and UILabel.

The UITableView works properly as long as there is no more than 9-10 cells displaying. But when the number of cells become higher, so the user has to scroll down to view them all, that's when the odd behavior begins. Content from the first, second, third and so on, is added to cell number eleven, twelve, thirteen and so on. And when the user then scroll up, the content that is supposed to be on number 11, 12, 13 is now in the first, second and third cell...

I hope someone understands my problem, and know what is wrong here..

Here is the code I user to add cells.. Ignore the parse stuff though, I dont think it is relevant

- (UITableViewCell *)tableView:(UITableView *)tableview cellForRowAtIndexPath:(NSIndexPath *)indexPath {
static NSString *simpleTableIdentifier = @"SimpleTableCell";

UITableViewCell *cell = [tableview dequeueReusableCellWithIdentifier:simpleTableIdentifier];

if (cell == nil) {
    cell = [[UITableViewCell alloc] initWithStyle:UITableViewCellStyleDefault reuseIdentifier:simpleTableIdentifier];

    if (tableview == commentViewTableView) {
        //Ignore this
    } else if (tableview == tableView) {
        UIImageView *imageView = [[UIImageView alloc] initWithFrame:CGRectMake(5, 5, 34, 34)];
        imageView.contentMode = UIViewContentModeScaleAspectFill;
        imageView.clipsToBounds = YES;
        [cell addSubview:imageView];

        UILabel *usernameLabel = [[UILabel alloc] initWithFrame:CGRectMake(44, 0, 160, 44)];
        usernameLabel.textAlignment = NSTextAlignmentLeft;
        usernameLabel.font = [UIFont systemFontOfSize:17];
        usernameLabel.backgroundColor = [UIColor clearColor];
        [cell addSubview:usernameLabel];


        UIImageView *hitImageView = [[UIImageView alloc] initWithFrame:CGRectMake(245, 9.5, 25, 25)];
        hitImageView.contentMode = UIViewContentModeScaleAspectFill;
        hitImageView.clipsToBounds = YES;
        hitImageView.image = [UIImage imageNamed:@"hit.png"];
        [cell addSubview:hitImageView];

        NSString *key = //Code to retrieve userKey

        PFQuery *query = [PFUser query];
        [query whereKey:@"objectId" equalTo:key];
        [query getFirstObjectInBackgroundWithBlock:^(PFObject *object, NSError *error) {
            if (!error) {
                [[object objectForKey:@"image1"] getDataInBackgroundWithBlock:^(NSData *data, NSError *error) {
                        if (!error) {

                            NSString *ageString = [[NSString alloc] initWithFormat:@"%li", (long)age];


                            imageView.image = [UIImage imageWithData:data];
                            usernameLabel.text = [NSString stringWithFormat:@"%@, %@", [object objectForKey:@"username"], ageString];

                        }
                    }];
            }
        }];
    }
}

return cell;

}

Answer 1

I solved my problem by doing changing the cell identifier to be unique. I don't know if this actually is the way to do it, or if it is good practice, but when I did it solved my problem. So it would be good with some feedback to know if this will cause any other problems I'm might be missing?

NSString *identifier = [NSString stringWithFormat:@"Cell%li", indexPath.row];
UITableViewCell *cell = [tableview dequeueReusableCellWithIdentifier:identifier];

if (cell == nil) {
    //My code..
}

Answer 2

There are a couple problems with the code. One is that special care must be taken with asynch calls inside the cellForRowAtIndex: datasource method. Another is that the cells are reused, so adding subviews to them each time they come into view will pile subviews upon subview.

Lets start with the asynch operation. @nburk correctly points out the issue, but its an overstatement to say you "can't do it". You could preload everything, but then user must wait for the whole table to be ready before they can see any of it. A good strategy here is lazy load.

Lazy load depends on a place to cache the loaded result. So lets make your datasource array an array of mutable dictionaries that look like this:

@{@"user": aPFUser, @"image": aUIImage };

It makes sense to prefetch the users, otherwise, you don't even know how many you have, so, in viewWillAppear:

// setup your model as @property(strong,nonatomic) NSMutableArray *users;
PFQuery *query = [PFUser query];
[query findObjectsInBackgroundWithBlock:^(NSArray *objects, NSError *error) {
    if (!error) {
        // build the datasource
        self.users = [NSMutableArray array];
        for (PFUser *user in objects) {
            NSMutableDictionary *d = [NSMutableDictionary dictionaryWithDictionary:
                @{ @"user": user };
            ];
        }
    [self.tableView reloadData];
    }
}];

Now, in cellForRowAtIndexPath you do this:

NSMutableDictionary *userDictionary = self.users[indexPath.row];

// in the lazy pattern, if the model is initialized, we're done
// start by assuming the best
imageView.image = userDictionary[@"image"];

// but the image might be nil (it will start out that way) so, load...
PFQuery *query = [PFUser query];
[query whereKey:@"objectId" equalTo:key];
[query getFirstObjectInBackgroundWithBlock:^(PFObject *object, NSError *error) {
    if (!error) {
        [[object objectForKey:@"image1"] getDataInBackgroundWithBlock:^(NSData *data, NSError *error) {
            if (!error) {
                UIImage *image = [UIImage imageWithData:data];

                // this is the important part:  this code doesn't run when the rest
                // of the method runs.  It happens later, when the request completes
                // so don't assume anything about the state of the table.  Instead
                // treat the table like you would in other view controller methods
                userDictionary[@"image"] = image;
                // don't fool around with cell (it might be reused).  Instead 
                // just tell the table to reload this row
                [tableView reloadRowsAtIndexPaths:@[indexPath]
                                 withRowAnimation:UITableViewRowAnimationAutomatic];
            }
        }];
    }
}];

The next time that row scrolls into view, the data from the asynch request will be cached in your user dictionary.

Problem two is simpler: the code builds subviews unconditionally, even if the (reused) cell already has that subview. The answer, again, is that laziness is your friend. Try to get the subview from the cell, and only build it if you must...

// change all of your subview-building code to do this:
UIImageView *imageView = (UIImageView *)[cell viewWithTag:32];
if (!imageView) {
   imageView = [[UIImageView alloc] init....
   // same code as you had here, adding...
   imageView.tag = 32;
}
// and so on for the cell's other subviews.  be sure to advance the tag (33, 34, etc)

In sum, the cellForRowAtIndexPath has a few sections.

• dequeue the cell
• lazy-build subviews as above
• as above: access your model and optimistically init the subviews from the model
• if part of the model is missing, do an asynch call, update the model, and reload the cell when done

Answer 3

Change your code like this:

- (UITableViewCell *)tableView:(UITableView *)tableview cellForRowAtIndexPath:(NSIndexPath *)indexPath {
   static NSString *simpleTableIdentifier = @"SimpleTableCell";

   UITableViewCell *cell = [tableview dequeueReusableCellWithIdentifier:simpleTableIdentifier];

   if (cell == nil) {
      cell = [[UITableViewCell alloc] initWithStyle:UITableViewCellStyleDefault reuseIdentifier:simpleTableIdentifier];
   } // CLOSED PARANTHESES HERE!!!!

   if (tableview == commentViewTableView) {
       //Ignore this
   } else if (tableview == tableView) {
      // ... rest of your code here
   }
 }