Retaining UITableViewCell selection statuses

Question

Why the UITableViewCells don't reload the checkmarks after selecting, scrolling away, then scrolling back?

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    #define CHECK_NULL_STRING(str) ([str isKindOfClass:[NSNull class]] || !str)?@"":str

    static NSString *CellIdentifier = @"inviteCell";
    UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:CellIdentifier forIndexPath:indexPath];

    cell.accessoryType = UITableViewCellAccessoryCheckmark;
    cell.textLabel.highlightedTextColor = [UIColor colorWithHexString:@"#669900"];
    cell.selectionStyle = UITableViewCellSelectionStyleGray;
    cell.backgroundColor = [UIColor blackColor];
    cell.textLabel.textColor = [UIColor whiteColor];
    [[UITableViewCell appearance] setTintColor:[UIColor colorWithHexString:@"#669900"]];

    if (cell == nil) {
        cell = [[UITableViewCell alloc] init];
    }

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

    BOOL isSearching = tableView != self.tableView;
    NSArray *arrayToUse = (isSearching ? searchResults : contactsObjects);
    id p = arrayToUse[indexPath.row];

    NSString *fName = (__bridge_transfer NSString *)(ABRecordCopyValue((__bridge ABRecordRef)(p), kABPersonSortByFirstName));
    NSString *lName = (__bridge_transfer NSString *)(ABRecordCopyValue((__bridge ABRecordRef)(p), kABPersonSortByLastName));
    cell.textLabel.text = [NSString stringWithFormat:@"%@ %@", CHECK_NULL_STRING(fName), CHECK_NULL_STRING(lName)];

    BOOL showCheckmark = [[stateArray objectAtIndex:indexPath.row] boolValue];
    if (showCheckmark == YES)
    {
        cell.accessoryType = UITableViewCellAccessoryCheckmark;
        NSLog(@"It hit showCheckmark = YES, and stateArray is %@",stateArray[indexPath.row]);
    }
    else
    {
        cell.accessoryType = UITableViewCellAccessoryNone;
        NSLog(@"It hit showCheckmark = NO, and stateArray is %@",stateArray[indexPath.row]);
    }

    return cell;
}

- (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath;
{
    id object = contactsObjects[indexPath.row];
    UITableViewCell *cell = [tableView cellForRowAtIndexPath:indexPath];

    if (cell.accessoryType == UITableViewCellAccessoryNone)
    {
        cell.accessoryType = UITableViewCellAccessoryCheckmark;
        [stateArray insertObject:[NSNumber numberWithBool:YES] atIndex:indexPath.row];
        [selectedObjects addObject:object];
    }
    else
    {
        cell.accessoryType = UITableViewCellAccessoryNone;
        [stateArray insertObject:[NSNumber numberWithBool:NO] atIndex:indexPath.row];
        [selectedObjects removeObject:object];
    }

    //slow-motion selection animation.
    [tableView deselectRowAtIndexPath:indexPath animated:YES];
}

Firmament

Have you verified that stateArray is being set correctly? – Honaker

I always try to work backwards from a known point of "disagreement". – Honaker

Check my edit. @Hot Licks – Firmament

Ahh. It appears that I need to be sending an object to the NSMutableArray, b/c BOOL is primitive. So I need to send an NSInteger to the stateArray – Firmament

I just tryed [stateArray insertObject:[NSNumber numberWithBool:YES] atIndex:indexPath.row]; in the else-if but it didn't work. – Firmament

Could it be because YES is not an object? Try @YES – Erato

That didn't work. I understand that this problem is totally because the BOOL value YES is not an object, I just can't figure out how to convert it to an object and properly store it in stateArray at indexPath.row – Firmament

I've even tried stateArray[indexPath.row] = [NSNumber numberWithBool:YES]; – Firmament

What do you see when you dump stateArray? insertObject:[NSNumber numberWithBool:YES] should work just fine. Or insertObject:@YES, since @YES produces an NSNumber of YES. – Honaker

When I dump stateArray, all results are: It hit showCheckmark = NO, and stateArray is (null) – Firmament

Can you upload the code somewhere? do you initialize stateArray? – Leonaleonanie

That's actually what the problem was. #21213932 The code was using insertObject when it needed to be using replaceObject.. also I can't really figure out why my stateArray NSMutableArray isn't usable in my didSelectRowAtIndexPath: method – Firmament

All that's left to do is properly instantiate stateArray. I've already made the property and synthesized it, and have even done a lazy instantiation, why isn't the variable localized to didSelectRowAtIndexPath:? – Firmament

Whoever down voted this, please change that. – Firmament

Answer 1

The cell selection problem was not solved, even when insertObject was replaced with replaceWithObject, however, one should not waste time setting BOOL objects with an NSInteger inside an NSMutableArray. Instead, for cell selection memory, one should use NSDictionary like this:

@property (nonatomic, strong) NSMutableDictionary * selectedRowCollection;

- (void)viewDidLoad{

    self.selectedRowCollection = [[NSMutableDictionary alloc] init];
}

- (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath;
{    
    id object = contactsObjects[indexPath.row];
    UITableViewCell *cell = [tableView cellForRowAtIndexPath:indexPath];

    if (cell.accessoryType == UITableViewCellAccessoryNone)
    {
        cell.accessoryType = UITableViewCellAccessoryCheckmark;
        [self.selectedRowCollection setObject:@"1" forKey:[NSString stringWithFormat:@"%d",indexPath.row]]; 
    }
    else
    {
        cell.accessoryType = UITableViewCellAccessoryNone;
        [self.selectedRowCollection removeObjectForKey:[NSString stringWithFormat:@"%d",indexPath.row]];
    }

    //slow-motion selection animation.
    [tableView deselectRowAtIndexPath:indexPath animated:YES];
}

   - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    BOOL showCheckmark =  [[self.selectedRowCollection valueForKey:[NSString stringWithFormat:@"%d",indexPath.row]] boolValue];

    if (showCheckmark == YES)
    {
        cell.accessoryType = UITableViewCellAccessoryCheckmark;
    }
    else
    {
        cell.accessoryType = UITableViewCellAccessoryNone;
    }
}

Firmament

Answer 2

You missed out the ! (inverse operator) on the following line meaning that the state will always be the same.

[stateArray replaceObjectAtIndex:indexPath.row withObject:[NSNumber numberWithBool:[[stateArray objectAtIndex:indexPath.row] boolValue]]];

It should be

[stateArray replaceObjectAtIndex:indexPath.row withObject:[NSNumber numberWithBool:![[stateArray objectAtIndex:indexPath.row] boolValue]]];

Edit --- I've refactored both methods because it can be done with a lot less code and it will completely simplify the methods for you.

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

    BOOL isSearching = tableView != self.tableView;
    NSArray *arrayToUse = (isSearching ? searchResults : contactsObjects);
    id p = arrayToUse[indexPath.row];

    NSString *fName = (__bridge_transfer NSString *)(ABRecordCopyValue((__bridge ABRecordRef)(p), kABPersonSortByFirstName));
    NSString *lName = (__bridge_transfer NSString *)(ABRecordCopyValue((__bridge ABRecordRef)(p), kABPersonSortByLastName));
    cell.textLabel.text = [NSString stringWithFormat:@"%@ %@", CHECK_NULL_STRING(fName), CHECK_NULL_STRING(lName)];

    BOOL showCheckmark = [stateArray[indexPath.row] boolValue];
    if (showCheckmark == YES) {
        cell.accessoryType = UITableViewCellAccessoryCheckmark;
    }
    else {
        cell.accessoryType = UITableViewCellAccessoryNone;
    }
    return cell;
}

- (void) tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath {
    id object = contactsObjects[indexPath.row];
    UITableViewCell *cell = [tableView cellForRowAtIndexPath:indexPath];
    if (cell.accessoryType == UITableViewCellAccessoryNone) {
        cell.accessoryType = UITableViewCellAccessoryCheckmark;
        [selectedObjects addObject:object];
    }
    else {
        cell.accessoryType = UITableViewCellAccessoryNone;
        [selectedObjects removeObject:object];
    }
    stateArray[indexPath.row] = @(cell.accessoryType == UITableViewCellAccessoryCheckmark);
}

Styles

do you mean on the else in didSelectRowAtIndexPath? – Firmament

@WhiteHatPrince That's correct. You could also store the cell you get from calling the method -cellForRowAtIndexPath in a variable so that you don't have to call the method twice. – Styles

Alright I just did that and the checkmarks are still disappearing upon scroll-and-scroll-back – Firmament

@WhiteHatPrince I've refactored the method and it should work perfectly now. – Styles

Thank you very much for the simplification @Styles – Firmament

I'm having a little bit of trouble understanding what the last line(of your simplification) is doing, the one containing stateArray – Firmament

Thanks to you again for the simplification. – Firmament

@WhiteHatPrince it replaces the NSNumber found in stateArray at the index indexPath.row and sets it to YES if the cell's accessoryType is a checkmark, and NO if it's not. – Styles

Oh.. very very nice. didn't know you could do that, I'm replacing the top method right now – Firmament

The use of == returns a boolean value. Think of it in the same way that an if statement is evaluated. – Styles

Awesome, but hum, those checkmarks are still deleting upon scroll. – Firmament

I actually gtg but i will brb – Firmament

@WhiteHatPrince I assume your arrays are nil then. – Styles

No. The names are already properly displaying in the uitableview – Firmament

I don't see the "following line" anywhere in the original post. – Honaker

@HotLicks the original post was changed to use the code in my answer, that's why you can't see it. – Styles

I looked at the edit history and did not see those lines. I guess I didn't look closely enough. – Honaker

Answer 3

I suggest a more object oriented approach. This will ensure that your code is flexible and displays correctly all the time.

For each item you wish to display in your table, have a corresponding object. You mentioned that you are displaying contacts, so let's suppose your object is called "Contact":

//Contact.h

@interface Contact : NSObject

@property BOOL selected;
@property NSString *name;

@end

//Contact.m
#import Contact.h
@implementation Contact

+ (id) contactWithName:(NSString*)name {
    Contact *nContact = [Contact new];
    nContact.name = name;
    nContact.selected = NO;
    return nContact;
}
@end

Then, just make your view work something like this:

//ContactView.m

@interface ContactView()

@property NSMutableArray *contacts;

@end

@implementation ContactView
@synthesize contacts;

- (void) viewDidLoad {
    [super viewDidLoad];
    //get your contact list here. When creating contacts, be sure to assign their selected and their name as you require.
    contacts = @[[Contact contactWithName:@"John"], [Contact contactWithName:@"Jane"]];
}

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath {
    static NSString *cellID = @"inviteCell";
    UITableViewCell *cell = [tableview dequeueReusableCellWithIdentifier:cellID];
    if (cell == nil) cell = [[UITableViewCell alloc] initWithStyle:UITableViewCellStyleDefault reuseIdentifier:cellID];

    Contact *cellContact = [contacts objectAtIndex:indexPath.row];
    cell.textLabel.text = cellContact.name;
    cell.accessoryType = cellContact.selected == YES ? UITableViewCellAccessoryCheckmark : UITableViewCellAccessoryNone;

    return cell;
}

- (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath {
    Contact *cellContact = [contacts objectAtIndex:indexPath.row];
    cellContact.selected = !cellContact.selected;
    [contacts replaceObjectAtIndex:indexPath.row withObject:cellContact];
    [tableView reloadData]; //to refresh without animation
    //[tableView reloadSections:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(0, [tableView numberOfSections])] withRowAnimation:UITableViewRowAnimationTop]; //to refresh with animation
}

@end

And boom, easy to use tables that always look right, queue properly, and are object oriented for easy maintenance later.

Snatch

I do like this, however this is changing a lot of what's already done. Definitely very easy to read and fix but I'm not sure this is about to solve the problem, and I understand my way perfectly fine. – Firmament

And the assignment of each contact would have to be something besides manual, as you did in the viewDidLoad: with contacts = @[[Contact contactWithName:@"John"], [Contact contactWithName:@"Jane"]];. Perhaps a for-loop could do that trick though – Firmament

Indeed, this was not meant to be snap-in code but rather to illustrate the concept you could use to make cleaner, easy to follow and fix code. As for solving the issue, it should- I do it this way specifically to prevent the issues you're having :) – Snatch

Answer 4

You are not doing this UITableViewCell *cell = [tableview dequeueReusableCellWithIdentifier:cellID]; if (cell == nil) {cell = [[UITableViewCell alloc] initWithStyle:UITableViewCellStyleDefault reuseIdentifier:cellID]; } Allocating is necessary if cell is nil. Or it cause problem while scrolling.

Septima

That's not true. If the cell was nil, the tableview would throw an exception. – Styles

+1 to the person above me- if it wasn't allocating cells it'd be crashing, not just having display issues. – Snatch