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.

In the didCheckTask method, can you put a NSLog at the beginning of the method and another one right before [checkList reloadData]. And the try to generate the error again. See if you can see which log did show before app crashes? — user523234, May 07 '12 at 04:41
Thank you for your help. I tried this. The first log printed, the second log (right before reloadData) did not print. It seems like the indexing is askew, but I can't for the life of me see where its happening. Is using the button tag reliable in this type of situation? (i.e. combined with editing)?? — mySilmaril, May 07 '12 at 18:20
It looked like the allTasks array has less items than your tableview after a deletion. You should put some NSLogs to verify after a deletion that your allTasks array has the correct items remained and agreed with your current display tableview. — user523234, May 07 '12 at 22:26
I just realized that in the cellForRowAtIndexpath, you did has this line: [[cell textLabel] setText:[NSString stringWithFormat:@"%@", [[[CLTaskStore sharedStore] allTasks] objectAtIndex:[indexPath row]]]]; So, what my previous post might not materialize! — user523234, May 07 '12 at 22:42

score 2 · Accepted Answer · edited May 23 '17 at 11:56

2

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.

edited May 23 '17 at 11:56

Community

1
1

answered May 09 '12 at 05:47

jrturton

118,105
32
252
268

Thank you! I thought this was probably a deadend two days ago, but I was told it could be done. Part of being a noob. I'm going to re-code using your suggestion. I'll let you know how it goes. – mySilmaril May 09 '12 at 17:31
A+, very nice. Thank you so much. – mySilmaril May 09 '12 at 18:13

Steve · Answer 2 · 2012-05-09T04:43:47.267

0

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.

edited May 09 '12 at 04:43

answered May 07 '12 at 06:13

Steve

1,840
17
20

Thank you for your help. I'm not sure I follow however. My CLChecklistViewController is both my delegate and datasource. When editing mode is entered, the message tableView:commitEditingStyle:forRowAtIndexPath: is sent to the datasource. In that method, I do two things: (1) remove the task from the CLTaskStore singleton and (2) remove the task from the table view. The method is called from the delete button *in* editing mode. Note that deletion works as expected MOST of the time. Its close, but not quite right. Again, when I delete the first item and then try to check the last item ...problem. – mySilmaril May 07 '12 at 18:15
Sorry, didn't see the code for that, so didn't know if you were deleting the record from the datasource. – Steve May 08 '12 at 01:54
I think, though, that your problem stems from the fact that your checkbox tags still have their original values. So when you try to check the last checkbox, it used to be the fifth cell in your tableview and thus has an indexPath row value of 4. When the cell is created, your checkbox tag value is set to that. Delete an item preceding that cell, and unless you renumber your checkbox tags, they will contain the original indexPath row values. The problem shows up only on the last item in the table because it's the only one whose index will cause a bounds violation. See edited answer. – Steve May 08 '12 at 01:58
Yes, you are absolutely right. See my edits above (looks like I'm checking in right behind you). The button's tag values must be off by one (Assuming a single deletion). Looking at your edits now ... hopefully I can fix my tags ... – mySilmaril May 08 '12 at 02:21
Hmm ... is there a *good* way to reconcile the indexing? I'm trying to re-index the allTasks array via my CLTaskStore to match the old button tags in the tableView:commitEditingStyle:forRowAtIndexPath method, but so far no joy. Is there an easier way to fox this? – mySilmaril May 08 '12 at 04:52
In removeTask: you're looping through right now to NSLog some properties. Instead, you could loop through from the index of the item being deleted to the end of the array, setting the tag equal to its current index. You'll have to do something similar when you move table entries around as well, otherwise their tags will point to their old location in the table. (Don't forget to checkmark this answer in SO.) – Steve May 08 '12 at 11:58
Thanks again Steve. I checked it for you, but I don't really have a solution yet. I don't understand how you can change the tags from the method you suggested. Could you possibly help me out with an example. I'm new at this and I'm stuck. Thank you so much! – mySilmaril May 09 '12 at 02:00
Re-indexing is the easy part. How do I access all of the buttons in each cell's accessory view to re-index their tags? This is the issue. I don't think it's possible to do what I'm trying to do using button tags. Please prove me wrong if your up for the challenge, but I'll need to see some actual code before I'll believe this can be done. – mySilmaril May 09 '12 at 17:26
Sorry to tease. I do appreciate your advice. This is probably possible, of course, but not without a lot of work. Jrturton's solution works poifectly – mySilmaril May 09 '12 at 18:17

Problems using custom button in UITableViewCell's Accessory View

2 Answers2