4
\$\begingroup\$

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 <QuartzCore/QuartzCore.h>
#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];
    [anAlert show];
}


- (void)alertView:(UIAlertView *)iAlertView willDismissWithButtonIndex:(NSInteger)iButtonIndex {
    if (iButtonIndex != [iAlertView cancelButtonIndex]) {
        NSString *aRoutineName = [iAlertView textFieldAtIndex:0].text;
        [self createRoutineWithName:aRoutineName];
    }
}


-(void)createRoutineWithName:(NSString *)iRoutineName {
    Routine *aRoutine = [DataModelController createRotuineWithName:iRoutineName];
    [self.routineTableView reloadData];
    [self pushToListForRoutine:aRoutine];
}


-(void)pushToListForRoutine:(Routine *)iRoutine {
    RoutineDayTableViewController *detailViewController = [[RoutineDayTableViewController alloc] initWithNibName:@"RoutineDayTableViewController" bundle:nil withRoutine:iRoutine];
    detailViewController.hidesBottomBarWhenPushed = YES;
    [self.navigationController pushViewController:detailViewController animated:YES];
}


#pragma mark - Table view data source

- (NSInteger)numberOfSectionsInTableView:(UITableView *)tableView {
    return 1;
}


- (NSInteger)tableView:(UITableView *)tableView numberOfRowsInSection:(NSInteger)section {
    id <NSFetchedResultsSectionInfo> sectionInfo = [[self.fetchedResultsController sections] objectAtIndex:section];
    return [sectionInfo numberOfObjects];
}


- (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;
}


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


- (void)setEditing:(BOOL)editing animated:(BOOL)animate {
    [self.routineTableView setEditing: !self.routineTableView.editing animated:YES];

    if (self.routineTableView.editing)
        [self.navigationItem.leftBarButtonItem setTitle:@"Done"];
    else
        [self.navigationItem.leftBarButtonItem setTitle:@"Edit"];
}


- (void)tableView:(UITableView *)tableView commitEditingStyle:(UITableViewCellEditingStyle)editingStyle forRowAtIndexPath:(NSIndexPath *)indexPath {
     if (editingStyle == UITableViewCellEditingStyleDelete) {
         NSManagedObjectContext *context = [self.fetchedResultsController managedObjectContext];
         [context deleteObject:[self.fetchedResultsController objectAtIndexPath:indexPath]];

         NSError *error = nil;

         if (![self.managedObjectContext save:&error]) {
             NSLog(@"error: %@", error);
         }
     }
}


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


#pragma mark - Table view delegate

- (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath {
    Routine *aRoutine = (Routine *)[self.fetchedResultsController objectAtIndexPath:indexPath];
    [self pushToListForRoutine:aRoutine];
    [tableView deselectRowAtIndexPath:indexPath animated:YES];
}


#pragma mark - Fetched results controller


- (NSFetchedResultsController *)fetchedResultsController {
    if (_fetchedResultsController != nil) {
        return _fetchedResultsController;
    }

    NSFetchRequest *fetchRequest = [[NSFetchRequest alloc] init];
    NSEntityDescription *entity = [NSEntityDescription entityForName:@"Routine" inManagedObjectContext:self.managedObjectContext];
    [fetchRequest setEntity:entity];

    NSSortDescriptor *sortDescriptor = [[NSSortDescriptor alloc] initWithKey:@"timeStamp" ascending:YES];
    [fetchRequest setSortDescriptors:[NSArray arrayWithObject:sortDescriptor]];

    [fetchRequest setFetchBatchSize:20];

    NSFetchedResultsController *theFetchedResultsController = [[NSFetchedResultsController alloc] initWithFetchRequest:fetchRequest managedObjectContext:self.managedObjectContext sectionNameKeyPath:nil cacheName:nil];
    self.fetchedResultsController = theFetchedResultsController;
    _fetchedResultsController.delegate = self;

    NSError *error = nil;
    if (![theFetchedResultsController performFetch:&error]) {
        NSLog(@"Unresolved error %@, %@", error, [error userInfo]);
        abort();
    }

    return _fetchedResultsController;
}


#pragma mark - Fetched results controller delegate


- (void)controllerWillChangeContent:(NSFetchedResultsController *)controller {
    [self.routineTableView beginUpdates];
}


- (void)controllerDidChangeContent:(NSFetchedResultsController *)controller {
    [self.routineTableView endUpdates];
    [self toggleEditButton];
}


- (void)controller:(NSFetchedResultsController *)controller didChangeSection:(id <NSFetchedResultsSectionInfo>)sectionInfo atIndex:(NSUInteger)sectionIndex forChangeType:(NSFetchedResultsChangeType)type {
    switch(type) {
        case NSFetchedResultsChangeInsert:
            [self.routineTableView insertSections:[NSIndexSet indexSetWithIndex:sectionIndex] withRowAnimation:UITableViewRowAnimationAutomatic];
            break;
        case NSFetchedResultsChangeDelete:
            [self.routineTableView deleteSections:[NSIndexSet indexSetWithIndex:sectionIndex] withRowAnimation:UITableViewRowAnimationAutomatic];
            break;
    }
}


- (void)controller:(NSFetchedResultsController *)controller didChangeObject:(id)anObject atIndexPath:(NSIndexPath *)indexPath forChangeType:(NSFetchedResultsChangeType)type newIndexPath:(NSIndexPath *)newIndexPath {
    UITableView *tableView = self.routineTableView;

    switch(type) {
        case NSFetchedResultsChangeInsert:
            [tableView insertRowsAtIndexPaths:[NSArray arrayWithObject:newIndexPath] withRowAnimation:UITableViewRowAnimationAutomatic];
            break;
        case NSFetchedResultsChangeDelete:
            [tableView deleteRowsAtIndexPaths:[NSArray arrayWithObject:indexPath] withRowAnimation:UITableViewRowAnimationAutomatic];
            break;
        case NSFetchedResultsChangeUpdate:
            [self configureCell:[tableView cellForRowAtIndexPath:indexPath] atIndexPath:indexPath];
            break;
        case NSFetchedResultsChangeMove:
            [tableView deleteRowsAtIndexPaths:[NSArray arrayWithObject:indexPath] withRowAnimation:UITableViewRowAnimationAutomatic];
            [tableView insertRowsAtIndexPaths:[NSArray arrayWithObject:newIndexPath]withRowAnimation:UITableViewRowAnimationAutomatic];
            break;
    }
}


@end
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.