Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

As per suggestion on SO, I am posting my code here to be reviewed. It was said I should not be saving and reading from disk so many times when a user selects and deselects a cell in my tableView.

@property (nonatomic, strong) IBOutlet UITableView *tableView;
@property (nonatomic, strong) NSArray *list;

- (void)viewDidLoad
{
    [super viewDidLoad];

    self.list = [[NSArray alloc] initWithObjects:@"Alabama", @"Alaska", @"Arizona", 
    @"Arkansas", @"California", @"Colorado", @"Connecticut", @"Delaware", @"District of Columbia",
    @"Florida", @"Georgia", @"Hawaii", @"Idaho", @"Illinois", @"Indiana", @"Iowa", @"Kansas", 
    @"Kentucky", @"Louisiana", @"Maine", @"Maryland", @"Massachusetts", @"Michigan", @"Minnesota",
    @"Mississippi", @"Missouri", @"Montana", @"Nebraska", @"Nevada", @"New Hampshire", 
    @"New Jersey", @"New Mexico", @"New York", @"North Carolina", @"North Dakota", @"Ohio",     @"Oklahoma",
    @"Oregon", @"Pennsylvania", @"Rhode Island", @"South Carolina", @"South Dakota", 
    @"Tennessee", @"Texas", @"Utah", @"Vermont", @"Virginia", @"Washington", @"West Virginia", 
    @"Wisconsin", @"Wyoming", nil];
}

- (NSString *)SettingsPlist
{
    NSString *paths = [NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES) objectAtIndex:0];
    NSString *PlistPath = [paths stringByAppendingPathComponent:@"Settings.plist"];
    return PlistPath;
}

- (NSInteger)tableView:(UITableView *)tableView
 numberOfRowsInSection:(NSInteger)section
{
    return [[self list] count];
}

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    NSString *contentForThisRow = [[self list] objectAtIndex:[indexPath row]];
    UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:@"Cell"];

    if(cell == nil)
    {
        cell = [[UITableViewCell alloc] initWithStyle:UITableViewCellStyleDefault reuseIdentifier:@"Cell"];
    }

    NSDictionary *dict = [[NSDictionary alloc] initWithContentsOfFile:[self SettingsPlist]];

    NSString *row = [NSString stringWithFormat:@"%d",indexPath.row];

    if([[dict objectForKey:row]isEqualToString:@"0"])
    {
        cell.accessoryType = UITableViewCellAccessoryNone;
    }
    else
    {
        cell.accessoryType = UITableViewCellAccessoryCheckmark;
    }

    [[cell textLabel] setText:contentForThisRow];

    return cell;
}

- (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath
{
    NSMutableDictionary *plist = [NSMutableDictionary dictionaryWithContentsOfFile:[self SettingsPlist]];

    UITableViewCell *cell = [tableView cellForRowAtIndexPath:indexPath];

    NSString *row = [NSString stringWithFormat:@"%d",indexPath.row];

    if(cell.accessoryType == UITableViewCellAccessoryNone)
    {
        cell.accessoryType = UITableViewCellAccessoryCheckmark;
        NSString *on = @"1";
        [plist setObject:on forKey:row];
        [plist writeToFile:[self SettingsPlist] atomically:YES];
    }
    else if(cell.accessoryType == UITableViewCellAccessoryCheckmark)
    {
        cell.accessoryType = UITableViewCellAccessoryNone;
        NSString *off = @"0";
        [plist setObject:off forKey:row];
        [plist writeToFile:[self SettingsPlist] atomically:YES];
    }
    [tableView deselectRowAtIndexPath:indexPath animated:YES];
}
share|improve this question

1 Answer 1

up vote 1 down vote accepted

you should read the settings plist once in -viewDidLoad: and save the contents as a dictionary to a property.

As you are doing, it is a huge performance hit as the disk will be accessed every time a cell appears on screen during scrolling.

In -tableView:cellForRowAtIndexPath: and -tableView:didSelectRowAtIndexPath: you then should only work on that dictionary.

Further more you could introduce a singleton-like shared object, that holds the settings and can be accessed in any class.

see my example code for how to do multiple selection and keeping track of the selected contents.

share|improve this answer
    
Save in the -viewWillDisappear: method? –  troop231 Dec 3 '12 at 3:51
    
yeah, that would be a good solution. –  vikingosegundo Dec 3 '12 at 3:52
    
So when the user selects and deselects a cell, do I need to keep track of that row with a boolean, and then write the boolean values to the plist? –  troop231 Dec 3 '12 at 3:53
    
you can change the contents in the dictionary immediately. to populate the cell correctly with the checkmark, see stackoverflow.com/a/13676588/106435 –  vikingosegundo Dec 3 '12 at 3:55
    
So you're saying it's safe to write to disk when the user selects and/or deselects cells? –  troop231 Dec 3 '12 at 3:59

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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