Search code examples
iosobjective-cuitableviewuiswitch

UITableView cells containing a UISwitch update inconsistently


I have a UITableView where each cell contains a UISwitch. The switch is supposed to toggle the content named in the cell as active or not, and it works perfectly when the user toggles the switch.

However, then I got the bright idea of also doing the toggling when the entire table row is selected. Simple in theory right? And it was, in terms of updating the data model.

However, for whatever reason that has had me stumped for the last 2 hours, only the switch in cell #3 updates to reflect the state change when I reload the table data. No matter how many rows I have in the table, it is always only the third cell that works. If there are less than 3 cells, then no cells work.

I've dumped a load of NSLog calls in there and they reflect the proper data states as I'd expect at all times, but they just aren't being reflected in the switch beyond the first load. If I leave the view controller then come back, the switches are all properly set...

I just know this is going to be some stupid thing I've done somewhere, because it works in exactly one instance, but for the life of me I cannot see it :(

Can you?

-(void)viewDidLoad
{
    [super viewDidLoad];

    [_tableAddPeople setDelegate:self];
    [_tableAddPeople setDataSource:self];
}

-(void)viewWillAppear:(BOOL)animated
{
    [_tableAddPeople reloadData];
}

-(void)listSwitchChanged:(id)sender
{
    UISwitch * switchControl = sender;

    Person * person = [SAVE_DATA getPersonWithIndex:(int)switchControl.tag];

    if (switchControl.on)
    {
        [SAVE_DATA activePeopleAdd:person.tag];
    }
    else
    {
        [SAVE_DATA activePeopleRemove:person.tag];
    }
}


#pragma mark UITableViewDataSource Delegate

-(NSInteger)tableView:(UITableView *)tableView numberOfRowsInSection:(NSInteger)section
{
    return [SAVE_DATA peopleCount];
}

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

    UILabel * nameLabel = (UILabel *)[cell.contentView viewWithTag:1];
    UISwitch * activeSwitch = (UISwitch *)[cell.contentView viewWithTag:2];

    Person * person = [SAVE_DATA getPersonWithIndex:(int)[indexPath row]];

    [nameLabel setText:[person name]];

    BOOL active = [SAVE_DATA activePeopleContains:person.tag];
    NSLog(@"Row for %@ is %@", person.name, (active? @"ON" : @"OFF"));
    [activeSwitch setOn:active animated:YES];
    [activeSwitch addTarget:self action:@selector(listSwitchChanged:) forControlEvents:UIControlEventValueChanged];
    activeSwitch.tag = (int)[indexPath row];

    return cell;
}

-(BOOL)tableView:(UITableView *)tableView canEditRowAtIndexPath:(NSIndexPath *)indexPath
{
    return NO;
}

-(BOOL)tableView:(UITableView *)tableView canMoveRowAtIndexPath:(NSIndexPath *)indexPath
{
    return NO;
}


#pragma mark UITableView Delegate

-(void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath
{
    Person * person = [SAVE_DATA getPersonWithIndex:(int)[indexPath row]];

    if ([SAVE_DATA activePeopleContains:person.tag])
    {
        NSLog(@"Removing %@", person.name);
        [SAVE_DATA activePeopleRemove:person.tag];
    }
    else
    {
        NSLog(@"Adding %@", person.name);
        [SAVE_DATA activePeopleAdd:person.tag];
    }

    [_tableAddPeople reloadData];
    //dispatch_async(dispatch_get_main_queue(), ^{[tableView reloadData];});
}

Solution

  • Your problem is that you are using the switch tag property for two things:

    1. To identify it in the view hierarchy (You have assigned value '2')
    2. To determine in which row the switch was changed

    Because you are assigning indexPath.row to tag, in the case where the index path isn't 2 (i.e. the third row), [cell.contentView viewWithTag:2]; will return nil when the cell is re-used.

    You also have an issue with cell re-use because you are adding the switch action handler multiple times.

    I would suggest that you adopt the following approach:

    1. Expose your cell's sub views using properties and cast the result of dequeueReusableCell to your UITableViewCell subclass so that you can access them without needing to use the tag

    2. Establish your UITableViewCell subclass as the switch action handler.

    3. Implement a delegation pattern between the cell and your view controller, so that the cell can notify the view controller that the switch changed. The cell can pass self to the delegate method. In the delegate method you can use indexPathForCell to determine the affected row.

    4. Don't reload the whole tableview just because one row has changed. Only reload the affected row.