Problems using custom button in UITableViewCell's Accessory View

Question

I'm building a simple checklist in a UITableView. I've added editing capability by placing the usual editing button in the navigation bar. The button turns on editing mode. Editing mode works great until I add custom check boxes (as buttons) in each cell's accessory view. I'm using this code to do it:

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    static NSString *CellIdentifier = @"Cell";

    UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:CellIdentifier];

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

        // put the tasks into the cell
        [[cell textLabel] setText:[NSString stringWithFormat:@"%@", [[[CLTaskStore sharedStore] allTasks] objectAtIndex:[indexPath row]]]];

        // put the checkbox into the cell's accessory view
        UIButton *checkBox = [UIButton buttonWithType:UIButtonTypeCustom];
        [checkBox setImage:[UIImage imageNamed:@"checkbox.png"] forState:UIControlStateNormal];
        [checkBox setImage:[UIImage imageNamed:@"checkbox-checked.png"] forState:UIControlStateSelected];
        checkBox.frame = CGRectMake(0, 0, 30, 30);
        checkBox.userInteractionEnabled = YES;
        [checkBox addTarget:self action:@selector(didCheckTask:) forControlEvents:UIControlEventTouchDown];
        cell.accessoryView = checkBox;

        // put the index path in the button's tag
        checkBox.tag = [indexPath row];
    }
    return cell;
}

As you can see, I'm using the button's tag to pass the indexPath to my didCheckTask: method:

- (void)didCheckTask:(UIButton *)button
{
    task = [[[CLTaskStore sharedStore] allTasks] objectAtIndex:button.tag];
    task.didComplete = YES;

    // toggle checkbox
    button.selected = !button.selected;

    [checkList reloadData];
}

The checkboxes and editing all seem to be working properly on the surface. However, a big problem arises when I enter editing mode, delete an item in the tableView and then try to use a checkbox. For example, if I delete the first item in the tableView and then try to check the last item's checkbox, the program crashes with:

2012-05-06 21:45:40.645 CheckList[16022:f803] * Terminating app due to uncaught exception 'NSRangeException', reason: '* -[__NSArrayM objectAtIndex:]: index 4 beyond bounds [0 .. 3]'

I have been trying to figure out the source of this bug, but I'm having no luck. I could really use some help - I'm new to cocoa. Pertinent code follows.

CLTaskFactory.h

#import <Foundation/Foundation.h>

@interface CLTaskFactory : NSObject
{
    NSString *taskName;
    BOOL didComplete;
}

@property NSString *taskName;

- (void)setDidComplete:(BOOL)dc;
- (BOOL)didComplete;

@end

CLTaskFactory.m

#import "CLTaskFactory.h"

@implementation CLTaskFactory

@synthesize taskName;

- (void)setDidComplete:(BOOL)dc
{
    didComplete = dc;
}

- (BOOL)didComplete
{
    return didComplete;
}

- (NSString *)description
{
    // override the description
    NSString *descriptionString = [[NSString alloc] initWithFormat:@"%@", taskName];
    return descriptionString;
}

@end

CLTaskStore.h

#import <Foundation/Foundation.h>

@class CLTaskFactory;

@interface CLTaskStore : NSObject
{
    NSMutableArray *allTasks;
}

+ (CLTaskStore *)sharedStore;

- (NSMutableArray *)allTasks;
- (void)addTask:(CLTaskFactory *)task;
- (void)removeTask:(CLTaskFactory *)task;
- (void)moveTaskAtIndex:(int)from toIndex:(int)to;

@end

CLTaskStore.m

    #import "CLTaskStore.h"

    @implementation CLTaskStore

    + (id)allocWithZone:(NSZone *)zone
    {
        return [self sharedStore];
    }

    + (CLTaskStore *)sharedStore
    {
        static CLTaskStore *sharedStore = nil;
        if (!sharedStore) {
            sharedStore = [[super allocWithZone:nil] init];
        }
        return sharedStore;
    }

    - (id)init
    {
        self = [super init];
        if (self) {
            allTasks = [[NSMutableArray alloc] init];
        }
        return self;
    }

    - (NSMutableArray *)allTasks
    {
        return allTasks;
    }

    - (void)addTask:(CLTaskFactory *)task
    {
        [allTasks addObject:task];
    }

    - (void)removeTask:(CLTaskFactory *)task
    {
        [allTasks removeObjectIdenticalTo:task];

        NSInteger taskCount = [allTasks count];
        NSLog(@"Removed: %@, there are now %d remaining tasks, they are:", task, taskCount);
        for (int i = 0; i < taskCount; i++) {
            NSLog(@"%@", [[[CLTaskStore sharedStore] allTasks] objectAtIndex:i]);
        }
    }

    - (void)moveTaskAtIndex:(int)from toIndex:(int)to
    {
        if (from == to) {
            return;
        }

        CLTaskFactory *task = [allTasks objectAtIndex:from];
        [allTasks removeObjectAtIndex:from];
        [allTasks insertObject:task atIndex:to];
    }

    @end


CLChecklistViewController.h

    #import <Foundation/Foundation.h>

    @class CLTaskFactory;

    @interface CLCheckListViewController : UIViewController
    {
        CLTaskFactory *task;
    }

    - (void)didCheckTask:(UIButton *)button;

    @end

CLCheckListViewController.m

#import "CLCheckListViewController.h"
#import "CLTaskFactory.h"
#import "CLTaskStore.h"

@implementation CLCheckListViewController
{
    __weak IBOutlet UITableView *checkList;
}

- (id)init
{
    self = [super init];
    if (self) {
        // add five sample tasks
        CLTaskFactory *task1 = [[CLTaskFactory alloc] init];
        [task1 setTaskName:@"Task 1"];
        [task1 setDidComplete:NO];
        [[CLTaskStore sharedStore] addTask:task1];

        CLTaskFactory *task2 = [[CLTaskFactory alloc] init];
        [task2 setTaskName:@"Task 2"];
        [task2 setDidComplete:NO];
        [[CLTaskStore sharedStore] addTask:task2];

        CLTaskFactory *task3 = [[CLTaskFactory alloc] init];
        [task3 setTaskName:@"Task 3"];
        [task3 setDidComplete:NO];
        [[CLTaskStore sharedStore] addTask:task3];

        CLTaskFactory *task4 = [[CLTaskFactory alloc] init];
        [task4 setTaskName:@"Task 4"];
        [task4 setDidComplete:NO];
        [[CLTaskStore sharedStore] addTask:task4];

        CLTaskFactory *task5 = [[CLTaskFactory alloc] init];
        [task5 setTaskName:@"Task 5"];
        [task5 setDidComplete:NO];
        [[CLTaskStore sharedStore] addTask:task5];
    }
    return self;
}

- (void)viewDidLoad
{
    [[self navigationItem] setTitle:@"Checklist"];

    // create edit button
    [[self navigationItem] setLeftBarButtonItem:[self editButtonItem]];
}

- (NSInteger)tableView:(UITableView *)tableView numberOfRowsInSection:(NSInteger)section
{
    return [[[CLTaskStore sharedStore] allTasks] count];
}

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    static NSString *CellIdentifier = @"Cell";

    UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:CellIdentifier];

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

        // put the tasks into the cell
        [[cell textLabel] setText:[NSString stringWithFormat:@"%@", [[[CLTaskStore sharedStore] allTasks] objectAtIndex:[indexPath row]]]];

        // put the checkbox into the cell's accessory view
        UIButton *checkBox = [UIButton buttonWithType:UIButtonTypeCustom];
        [checkBox setImage:[UIImage imageNamed:@"checkbox.png"] forState:UIControlStateNormal];
        [checkBox setImage:[UIImage imageNamed:@"checkbox-checked.png"] forState:UIControlStateSelected];
        checkBox.frame = CGRectMake(0, 0, 30, 30);
        checkBox.userInteractionEnabled = YES;
        [checkBox addTarget:self action:@selector(didCheckTask:) forControlEvents:UIControlEventTouchDown];
        cell.accessoryView = checkBox;

        // put the index path in the button's tag
        checkBox.tag = [indexPath row];
    }
    return cell;
}

- (void)didCheckTask:(UIButton *)button
{
    task = [[[CLTaskStore sharedStore] allTasks] objectAtIndex:button.tag];
    task.didComplete = YES;

    // toggle checkbox
    button.selected = !button.selected;

    [checkList reloadData];
}

- (void)setEditing:(BOOL)editing animated:(BOOL)animated
{
    [super setEditing:editing animated:animated];

    // set editing mode
    if (editing) {
        self.navigationItem.title = @"Edit Checklist";
        [checkList setEditing:YES];
    } else {
        self.navigationItem.title = @"Checklist";
        [checkList setEditing:NO];
    }
}

- (void)tableView:(UITableView *)tableView commitEditingStyle:(UITableViewCellEditingStyle)editingStyle
                                            forRowAtIndexPath:(NSIndexPath *)indexPath
{
    // remove guest from store
    if (editingStyle == UITableViewCellEditingStyleDelete) {

        task = [[[CLTaskStore sharedStore] allTasks] objectAtIndex:[indexPath row]];
        [[CLTaskStore sharedStore] removeTask:task];

        // remove guest from table view
        [checkList deleteRowsAtIndexPaths:[NSArray arrayWithObject:indexPath] withRowAnimation:UITableViewRowAnimationFade];
    }
}

- (void)tableView:(UITableView *)tableView moveRowAtIndexPath:(NSIndexPath *)sourceIndexPath toIndexPath:(NSIndexPath *)destinationIndexPath
{
    [[CLTaskStore sharedStore] moveTaskAtIndex:[sourceIndexPath row] toIndex:[destinationIndexPath row]];
}

@end

Thank you so much for your help and expertise!

edited:

I modified two methods with looping NSLogs to gain some insight. First, CLTaskStore:

- (void)removeTask:(CLTaskFactory *)task
{
    [allTasks removeObjectIdenticalTo:task];

    NSInteger taskCount = [allTasks count];
    NSLog(@"Removed: %@, there are now %d remaining tasks, they are:", task, taskCount);
    for (int i = 0; i < taskCount; i++) {
        NSLog(@"%@, status: %@", [[[CLTaskStore sharedStore] allTasks] objectAtIndex:i], [[[[CLTaskStore sharedStore] allTasks] objectAtIndex:i] didComplete]?@"YES":@"NO");
    }
}

Second, CLTaskListViewController:

- (void)didCheckTask:(UIButton *)button
{
    task = [[[CLTaskStore sharedStore] allTasks] objectAtIndex:button.tag];
    task.didComplete = YES;

    NSInteger taskCount = [[[CLTaskStore sharedStore] allTasks] count];
    for (int i = 0; i < taskCount; i++) {
        NSLog(@"%@, status: %@", [[[CLTaskStore sharedStore] allTasks] objectAtIndex:i], [[[[CLTaskStore sharedStore] allTasks] objectAtIndex:i] didComplete]?@"YES":@"NO");
    }

    // toggle checkbox
    button.selected = !button.selected;

    [checkList reloadData];
}

I noticed two things. If I delete upwards, from bottom to top, there are no issues. I can check anything - everything works. However, if I delete the first row and then check the last row the program crashes. The NSLog from the deletion is clean, its working fine.

If I delete the first row and check the fourth row, the NSLog from CLTaskStore reports row 5 was checked.

This is the problem. The two are definitely out of sequence after the deletion.

Answer 1

Your entire problem stems from the bad idea of using tags to indicate what row a button is in. This is bad enough when you aren't deleting rows from the datasource, but when you are, this is the sort of problem you can run into.

Using the location of the tapped item in the table view, and getting the index path of the location from the table view, is far more robust and works with editable tables and multi-section tables. See sample code in my answer here.

If you do it that way there is no re-indexing necessary.

Answer 2

When the delete button is pressed after entering Edit mode for your tableView, you must remove the corresponding data item from the datasource. Your code shows that you have a removeTask: method, but I don't see where you are actually calling that method to delete the corresponding task entry from your datasource. A good place to do this would be in the tableview:commitEditingStyle:forRowAtIndexPath: method in your view controller.

Since you are deleting the corresponding item in the datasource, further study of the code shows that your checkbox tag values still have their original values. If you delete any tableView item before the last one, then try to check the last one, your didCheckTask method tries to access the original indexPath row value, which now does not exist and causes a bounds exception. If you delete the first two cells, then the last two tableView items will both cause exceptions, and so on.

It wouldn't work in the didCheckTask method, but in the removeTask: method, after you delete the object from your datasource, loop through the remaining objects and set each tag equal to its corresponding array index. In the moveTaskAtIndex:toIndex: method, after you move your array entries around due to the user reordering items, do the same thing -- loop through the array and set each tag equal to its index in the array.