NSFetchedResultsController implementation

I'm using both Core Data and NSFetchedResultsController. Is there anything I can improve on?

```
//
// RoutineTableViewController.m
// App
//
// Created by Me on 3/21/11.
// Copyright 2011 __MyCompanyName__. All rights reserved.
//

#import "RoutineTableViewController.h"
#import
#import "FlurryAnalytics.h"
#import "RoutineDayTableViewController.h"
#import "Routine.h"
#import "Exercise.h"
#import "DataModelController.h"

@implementation RoutineTableViewController

• (void)viewDidAppear:(BOOL)animated {

[super viewDidAppear:YES];
self.fetchedResultsController = nil;
[self.fetchedResultsController performFetch:nil];
[self toggleEditButton];
}

• (void)viewDidLoad {

[super viewDidLoad];

[FlurryAnalytics logEvent:@"Routine"];

if (!self.managedObjectContext) {
self.managedObjectContext = [(MyAppDelegate *)[[UIApplication sharedApplication] delegate] managedObjectContext];
}

self.navigationItem.title = @"Routines";
self.routineTableView.rowHeight = 65;
self.routineTableView.delegate = self;
self.routineTableView.backgroundColor = [UIColor clearColor];
self.view.backgroundColor = [UIColor myBackgroundColor];

self.navigationItem.leftBarButtonItem = self.editButtonItem;
self.navigationItem.rightBarButtonItem = [[UIBarButtonItem alloc] initWithBarButtonSystemItem:UIBarButtonSystemItemAdd target:self action:@selector(presentCreateRoutineAlert)];

[self toggleEditButton];
}

• (void)toggleEditButton {

if (!self.fetchedResultsController.fetchedObjects.count > 0) {
self.navigationItem.leftBarButtonItem.enabled = NO;
} else {
self.navigationItem.leftBarButtonItem.enabled = YES;
}
}

-(void)presentCreateRoutineAlert {
UIAlertView *anAlert = [[UIAlertView alloc]initWithTitle:@"Add Routine" message:@"Enter name for routine" delegate:self cancelButtonTitle:@"Cancel" otherButtonTitles:@"Create", nil];
[anAlert setAlertViewStyle:UIAlertViewStylePlainTextInput]

Just briefly looking at your table view controller code, I'd in general suggest a more efficient way of configuring your cells. You have:

- (void)configureCell:(UITableViewCell *)cell atIndexPath:(NSIndexPath *)indexPath {
    cell.imageView.image = [UIImage imageNamed:@"44-shoebox.png"];
    cell.accessoryType = UITableViewCellAccessoryDisclosureIndicator;
    cell.backgroundView = [[UIImageView alloc] initWithImage:[UIImage imageNamed:@"newcell.png"]];
    cell.textLabel.backgroundColor = [UIColor clearColor];
    cell.detailTextLabel.backgroundColor = [UIColor clearColor];
    cell.textLabel.font = [UIFont textLabelFontBig];

    Routine *aRoutine = (Routine *)[self.fetchedResultsController objectAtIndexPath:indexPath];
    cell.textLabel.text = aRoutine.name;
}

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

    UITableViewCell *cell = [self.routineTableView dequeueReusableCellWithIdentifier:CellIdentifier];

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

    [self configureCell:cell atIndexPath:indexPath];
    return cell;
}

This means that configureCell:atIndexPath: will be run every time a cell is requested, which happens whenever the user scrolls the table. Most of the work in that method only needs to be done when new cells are created, which won't necessarily even happen during every user scroll (since cells are recycled).

... especially since you're creating a new UIImageView in that method, which isn't necessary. It looks like only this line:

cell.textLabel.text = aRoutine.name;

needs to be run every time a cell is requested, since the name can change. So, I would move that code out of configureCell:atIndexPath:, and change your code to this:

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

        UITableViewCell *cell = [self.routineTableView dequeueReusableCellWithIdentifier:CellIdentifier];

        if (!cell) {
            cell = [[UITableViewCell alloc] initWithStyle:UITableViewCellStyleSubtitle reuseIdentifier:CellIdentifier];
            [self configureCell:cell atIndexPath:indexPath];
        }

        // update the cell label for this specific routine name:
        Routine *aRoutine = (Routine *)[self.fetchedResultsController objectAtIndexPath:indexPath];
        cell.textLabel.text = aRoutine.name;

        return cell;
    }

Now, with the way we're using configureCell:atIndexPath:, it might be clearer to rename it to initializeCell:atIndexPath: or something. Also, if you have too much more code in that method, I might suggest making a new subclass of UITableViewCell, and just putting all that code into its initWithStyle:reuseIdentifier: method, so you can remove the call to configureCell:atIndexPath: from the table view controller completely.

But, that would be more of a code modularity / cleanliness issue. The first point I made about not calling configureCell:atIndexPath: every time can actually be a performance issue.

StackExchange Code Review Q#23769, answer score: 2